diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 7d398f7..b5d5c65 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "vstack", - "version": "1.8.0", + "version": "1.9.1", "source": "./claude", "description": "26 skills that fire without a slash command, 8 agents, 14 commands, and the session hook that routes situations to skills. Most skills are ported from pstack and Superpowers — see claude/skills/ATTRIBUTION.md for per-skill source and license.", "category": "workflow" diff --git a/.claude/verify.sh b/.claude/verify.sh index 7a5e255..d3710a4 100755 --- a/.claude/verify.sh +++ b/.claude/verify.sh @@ -35,7 +35,7 @@ for t in jq git; do command -v "$t" >/dev/null 2>&1 || missing="$missing $t"; do # --- 1. every shell script parses ---------------------------------------------------------- errs="" while IFS= read -r f; do - head -1 "$f" | grep -q '^#!.*sh' || continue + grep -q '^#!.*sh' <<<"$(head -1 "$f" 2>/dev/null)" || continue # not a pipe: SIGPIPE + pipefail = 141 out=$(bash -n "$f" 2>&1) || errs="$errs\n$f: $out" done < <(find . -path ./.git -prune -o -type f \( -name "*.sh" -o -path "./bin/*" \) -print) [ -z "$errs" ] && ok "shell syntax" || bad "shell syntax" "$(printf '%b' "$errs")" @@ -345,6 +345,13 @@ command -v jq >/dev/null && nmc=$(jq 'keys|length' mcp/servers.json 2>/dev/null # at 25 against a gate of 26 and nothing could see it, because the noun was not in the map. nck=$TOTAL +# Same shape, one noun later. The CHANGELOG said "29 shell scripts" against a tree of 27 .sh +# files and 31 shebang scripts, and nothing could see it because "shell script" was not in the +# map. Derived the same way check 29 selects, so the two can never disagree. +nsh=$(git ls-files 2>/dev/null | while IFS= read -r f; do + grep -q '^#!.*sh' <<<"$(head -1 "$f" 2>/dev/null)" && printf 'x\n' +done | grep -c .) + want_for(){ # noun (lowercased, plural or singular) -> expected count, or empty if not covered case "$1" in skill|skills) printf '%s' "$nsk" ;; @@ -355,6 +362,7 @@ want_for(){ # noun (lowercased, plural or singular) -> expected count, or empty "cli wrapper"|"cli wrappers") printf '%s' "$nwr" ;; case|cases) printf '%s' "$ncs" ;; "mcp server"|"mcp servers") printf '%s' "$nmc" ;; + "shell script"|"shell scripts") printf '%s' "$nsh" ;; esac } @@ -369,11 +377,40 @@ exempt_phrases(){ esac } +# Every noun this check can resolve, spelled once. The extraction regex is built from this +# list, so a noun can no longer be resolvable-but-never-extracted -- which is how "29 shell +# scripts" sat in the CHANGELOG unchallenged. want_for had no case for it, and even after one +# was added the claim stayed invisible, because the extractor carried its own separate +# alternation and nothing compared the two. +NOUNS='skills?|checks?|agents?|subagents?|sub-agents?|commands?|hooks?|CLI wrappers?|cases?|MCP servers?|shell scripts?' + errs="" -for f in README.md .claude-plugin/marketplace.json claude/.claude-plugin/plugin.json \ + +# Positive control. Every alternative the extractor looks for must resolve to a number, or the +# check pulls claims out of the docs and then drops them on the floor without saying so. +while IFS= read -r _n; do + [ -n "$_n" ] || continue + [ -n "$(want_for "$(printf '%s' "$_n" | tr '[:upper:]' '[:lower:]')")" ] \ + || errs="$errs\ninternal: the extractor looks for '$_n' but want_for cannot resolve it" +done </dev/null) + norm=$(awk -v v="$_cv" '/^## /{ sec = ($2 == "Unreleased" || index($2, v) == 1) } sec' "$f" \ + | tr '\n' ' ' | tr -s '[:space:]' ' ') + else + norm=$(tr '\n' ' ' < "$f" | tr -s '[:space:]' ' ') + fi while IFS= read -r ph; do [ -n "$ph" ] && norm=${norm//"$ph"/} done </dev/null && command -v jq >/dev/null; then # quickstart pinned v1.4.0 while the manifests said v1.8.0, so anyone copy-pasting the "pin a # release" lane got a four-version-old payload and no error -- the tag resolves, the install # succeeds, and the only symptom is a setup that quietly disagrees with its own README. + # + # Agreeing with the manifest is not enough. The quickstart's "pin a release" lane pinned + # v1.8.0 while the manifests said v1.8.0 and no such tag existed, so the check was satisfied + # and the URL a stranger copy-pastes returned 404. A pin has to name a tag that is actually + # there. Only asserted where the checkout has tags at all -- a shallow clone has none, and the + # branch below already declines to measure in that case. pins="" for f in README.md docs/*.md; do [ -f "$f" ] || continue while IFS= read -r pv; do - [ -n "$pv" ] && [ "$pv" != "$mv_" ] && pins="$pins\n $f pins v$pv" + [ -n "$pv" ] || continue + if [ "$pv" != "$mv_" ]; then + pins="$pins\n $f pins v$pv" + elif [ -n "$(git tag -l 2>/dev/null | head -1)" ] \ + && ! git rev-parse -q --verify "refs/tags/v$pv" >/dev/null 2>&1; then + pins="$pins\n $f pins v$pv, which is not a tag in this repository (the URL 404s)" + fi done </dev/null | sed -E 's/.*v//' | sort -u) PINEOF @@ -961,7 +1010,12 @@ PINEOF # failure mode it was written to prevent, reproduced inside itself. skip "declared version matches what installs" "no tags in this checkout (shallow clone?), so there is nothing to compare against" elif ! git rev-parse -q --verify "refs/tags/v$mv_" >/dev/null 2>&1; then - ok "declared version matches what installs (v$mv_ not yet tagged)" + # A declared-but-untagged version has no payload to diff against, so this branch compares + # nothing. It used to print "ok", which is the same defect the tagless branch above guards + # against, one elif lower down: a green that measured nothing, hidden from the skip census + # because only skips are counted there. Say skip, and the release unit has to tag before the + # check starts measuring again. + skip "declared version matches what installs" "v$mv_ is declared by the manifests but not tagged, so there is no payload to compare it against — tag the release and this starts measuring" else # Everything a lane actually delivers. Docs, tests and CI are deliberately excluded: they # change without changing what a stranger receives. @@ -1148,24 +1202,100 @@ fi # --- 29. every shell script passes shellcheck ------------------------------------------------ # -# This bundle is 27 shell scripts and almost nothing else, and the whole product is the claim +# This bundle is shell scripts and almost nothing else, and the whole product is the claim # that they behave correctly on someone else's machine. The class of bug that keeps landing here # is not exotic -- an unquoted expansion, a pattern that can never match, a variable set for a # check nobody wrote -- and a linter finds all three for free. # # Warning level, not style: informational notes are opinions and this should fail on defects. -# Where a warning is wrong the suppression carries a reason on the line above it, so the next -# reader can see the argument rather than a bare disable. +# Where a warning is wrong the suppression carries a reason, which check 30 enforces. +# +# Selected by shebang, the same way check 1 selects. It used to be the hand-maintained list +# `git ls-files '*.sh' bin/doctor bin/vstack`, and bin/cloudflare-mcp -- a #!/bin/sh script with +# no .sh suffix -- had never been on it. Appending an unquoted `$HOME/some path` to that file +# left shellcheck exiting 1 on it while this check still printed "ok shellcheck clean (29 +# scripts)". A list you have to remember to update is a list that goes stale silently. if command -v shellcheck >/dev/null 2>&1; then - sc_out=$(git ls-files '*.sh' bin/doctor bin/vstack 2>/dev/null \ - | while IFS= read -r f; do shellcheck -S warning -f gcc "$f" 2>/dev/null; done) + sc_files=$(git ls-files 2>/dev/null | while IFS= read -r f; do + grep -q '^#!.*sh' <<<"$(head -1 "$f" 2>/dev/null)" && printf '%s\n' "$f" + done) + sc_out=$(while IFS= read -r f; do + [ -n "$f" ] && shellcheck -S warning -f gcc "$f" 2>/dev/null + done <<<"$sc_files") [ -z "$sc_out" ] \ - && ok "shellcheck clean ($(git ls-files '*.sh' bin/doctor bin/vstack 2>/dev/null | wc -l | tr -d ' ') scripts, warning level)" \ + && ok "shellcheck clean ($(grep -c . <<<"$sc_files") scripts, warning level)" \ || bad "shellcheck clean" "$(printf '%s' "$sc_out" | sed 's/^/ /' | head -20)" else skip "shellcheck clean" "shellcheck not installed (brew install shellcheck / apk add shellcheck)" fi +# --- 30. every shellcheck suppression carries a reason ---------------------------------------- +# +# Check 29's own header has said for several versions that a suppression carries its reason with +# it, so the next reader sees the argument rather than a bare disable. Nothing enforced it, and +# bootstrap.sh had carried a naked `# shellcheck disable=SC2086` since the lane was written. A +# rule that lives only in prose is a rule that gets skipped by whoever did not read the prose, +# which is the second time that has happened here -- the documented-count rule was the first. +# +# A reason counts if it is on the same line after the code list, which is how the other five +# suppressions in this repo are written, or on the line immediately above. Both are readable at +# the point of the disable; a reason three lines away is not. +bare="" +nsup=0 +while IFS= read -r f; do + [ -n "$f" ] || continue + grep -q '^#!.*sh' <<<"$(head -1 "$f" 2>/dev/null)" || continue + while IFS= read -r hit; do + [ -n "$hit" ] || continue + n=${hit%%:*} + nsup=$((nsup + 1)) + line=$(sed -n "${n}p" "$f") + # whatever follows the comma-separated code list on the same line + tail_=$(sed -E 's/.*shellcheck[[:space:]]+disable=[A-Za-z0-9,]+//' <<<"$line") + above=$(sed -n "$((n - 1))p" "$f") + if ! grep -qE '[A-Za-z]{3}' <<<"$tail_" && ! grep -qE '^[[:space:]]*#.*[A-Za-z]{3}' <<<"$above"; then + bare="$bare\n $f:$n" + fi + # A directive shellcheck honours is always its own comment line, so anchor on that. Matching + # the bare phrase also picked up this file's own prose about the rule and the mutation + # payload in tests/gate-falsifiability.sh, and reported 9 suppressions where there are 6. + done <<<"$(grep -nE '^[[:space:]]*#[[:space:]]*shellcheck[[:space:]]+disable=' "$f" 2>/dev/null)" +done <<<"$(git ls-files 2>/dev/null)" +[ -z "$bare" ] \ + && ok "shellcheck suppressions carry a reason ($nsup suppressions)" \ + || bad "shellcheck suppressions carry a reason" \ + "$(printf 'a bare disable hides the argument from the next reader:%b' "$bare")" + +# --- 31. every shipped file has a referrer ----------------------------------------------------- +# +# Check 28 does this for docs/, where a link to the containing directory counts. Everything else +# in the tree had no such rule, and two files had been riding along for versions: a launchd +# wrapper around the doctor that install.sh never installs and uninstall.sh never removes, and an +# eval-loop driver nothing but its own header mentioned. Neither was reachable and neither was +# visible to any check. They are not named here on purpose -- a basename in this comment is a +# referrer as far as the grep below is concerned, which is exactly how the first draft of this +# check passed over both of them. +# +# The limit, stated rather than hidden: a mention in prose counts. This finds files nothing points +# at, not files pointed at only rhetorically. +# +# Skills, agents, commands and issue templates are excluded because the loader finds them by +# directory: a referrer would be redundant there, not missing. docs/ is excluded because check 28 +# owns it with directory-link semantics this basename match cannot express. +unref="" +while IFS= read -r f; do + [ -n "$f" ] || continue + case "$f" in + claude/skills/*|claude/agents/*|claude/commands/*|.github/ISSUE_TEMPLATE/*|docs/*) continue ;; + esac + [ -n "$(git grep -l -F "${f##*/}" -- . ":(exclude)$f" 2>/dev/null | head -1)" ] \ + || unref="$unref\n $f" +done <<<"$(git ls-files 2>/dev/null)" +[ -z "$unref" ] \ + && ok "every shipped file has a referrer ($(git ls-files | grep -cvE '^(claude/(skills|agents|commands)/|\.github/ISSUE_TEMPLATE/|docs/)') outside the load-by-directory trees)" \ + || bad "every shipped file has a referrer" \ + "$(printf 'nothing in this repository names:%b\n delete it, or give it a referrer -- a file nobody can find is a file nobody maintains' "$unref")" + echo # Accounting. Every declared check must have reported either a result or a skip. A check # that throws a shell error mid-body, or is wrapped in a conditional with no else, silently diff --git a/CHANGELOG.md b/CHANGELOG.md index 5810f36..8c145cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,83 @@ Versions follow [semver](https://semver.org). The version lives in two manifests ## Unreleased +## 1.9.1 — 2026-08-22 + +**A fresh bootstrap ended on a red line.** setup-machine.sh installs claude-mem, bin/doctor has +checked that the plugin's UserPromptSubmit hooks are async for several versions, and nothing ever +set the flag. So the lane installed the plugin and then left the machine in a state its own doctor +called drift, telling the operator to re-apply something that had never been applied once. + +The install-matrix doctor-stranger case could not see it. That case exercises install.sh, and the +plugin only arrives through setup-machine.sh, which only the bootstrap lane runs. It was found by +running the README quickstart verbatim into a scratch HOME, which is the point of running it +verbatim rather than reading it. + +setup-machine.sh now sets the flag, idempotently: it reads first and rewrites only when the flag +is not already set, because claude-mem auto-updates rewrite hooks.json and revert it. Measured on +a scratch HOME: doctor goes from one red line and DRIFT to 23 ok, 0 red, 6 notes. + +**doctor --drift deleted an unpushed release tag.** It ran a bare `git fetch` in the vstack +checkout to work out how far behind the remote it was. A bare fetch is not read-only: it does +whatever ~/.gitconfig says, and with fetch.prune and fetch.pruneTags set true it deletes every +local tag and remote-tracking branch the remote does not have. During this audit it destroyed the +v1.9.1 tag seconds after it was created, and the release check then reported ok for a version +whose tag was already gone. + +Every flag is spelled out now, so ambient config cannot turn an inspection into an edit. The new +doctor-no-mutate case in the install matrix clones a real checkout, sets that config pairing +locally, plants an unpushed tag and asserts it survives. It carries three controls, because the +first version of the case bailed before reaching the fetch and passed against the unfixed doctor. + +v1.9.0 is tagged and describes a payload carrying the claude-mem defect. It is left in place +rather than moved, because a tag somebody may have fetched is not a thing to rewrite. Use v1.9.1. + +## 1.9.0 — 2026-08-22 + +An audit pass. Every finding below is a green that measured nothing, which is the fifth time +that class has shipped here, so each one leaves behind a check and a mutation row rather than +just a fix. + +**Check 24 said ok over a comparison it never ran.** A version declared by the manifests but not +yet tagged has no payload to diff against, and that branch printed ok. The tagless branch one +elif below already knew better. It now skips with a reason, so the skip census can see it. + +**A pinned quickstart that 404s.** The README's "pin a release" lane pinned v1.8.0, the manifests +said v1.8.0, and no such tag existed. The check compared the two strings, found them equal, and +was satisfied. Measured: HTTP 404. A pin now has to name a tag that is actually there. + +**shellcheck was linting a hand-maintained list.** `git ls-files '*.sh' bin/doctor bin/vstack` +never included bin/cloudflare-mcp, a #!/bin/sh script with no .sh suffix. An unquoted expansion +appended to it made shellcheck exit 1 while the gate printed "ok shellcheck clean (29 scripts)". +Selection is by shebang now, the way check 1 already did it, and the count is 30. Row 29 mutates +that file specifically, so it proves the linter runs over everything rather than that it runs. + +**The count check dropped nouns on the floor.** `want_for()` resolved eight nouns and the +extractor carried a separate grep alternation, so a claim could be extractable-but-unresolvable +or the reverse, silently either way. Both come from one list now, with a positive control that +fails if the extractor looks for a noun `want_for` cannot resolve. Adding "shell scripts" +surfaced a stale CHANGELOG claim, and CHANGELOG's current-version section is now in the scan. + +**A suppression-reason rule that only lived in a comment.** Check 29's header had claimed it for +several versions while bootstrap.sh carried a naked disable=SC2086. Check 30 enforces it. + +**Two files nothing pointed at.** A launchd wrapper around the doctor, which install.sh never +installed and uninstall.sh never removed, and the eval-loop driver. The wrapper is deleted; the +driver now has a real referrer in tests/README.md. Check 31 makes an unreferenced file a failure. + +**An uninstall that left Conductor pinning policy.** install.sh writes ~/.conductor/settings.toml +and settings.managed.toml; uninstall.sh had no reference to conductor at all, so both survived +removal, and the managed file is the one that pins models and plan mode. + +**The 141 that hid all of it.** tests/gate-falsifiability.sh probed for a check's skip with +`verify.sh | grep -q`. Under `set -o pipefail` grep -q exits on the first match, verify dies of +SIGPIPE, and the pipeline returns 141, which reads as "did not skip". Measured: rc=141 with +pipefail, rc=0 without. Four sites now capture first and grep a here-string. + +The gate is 33 checks. Two of the new ones defeated themselves before they worked: naming a file +in the check that hunts unnamed files gives it a referrer, and so does naming the probe in the +mutation row. + ## 1.8.0 — 2026-08-22 **Two skill routings are mandatory now, not merely instructed.** Everything vstack did to route @@ -26,7 +103,7 @@ names in CI and the platform names in the README to be the same set, in both dir Codex, not a local model behind a compatibility shim. Every mechanism here is Claude Code's own and there is no adapter layer. -**shellcheck is a gate.** This bundle is 29 shell scripts and almost nothing else. Warning level, +**shellcheck is a gate.** This bundle is shell scripts and almost nothing else. Warning level, and where a warning is wrong the suppression carries its reason on the line above. It found a pattern in the destructive guard that could never match, a variable in `doctor` computed for a check nobody ever wrote, and two dead assignments. diff --git a/README.md b/README.md index 9c2b7b9..f017220 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ That claim is easy to make and most setups make it. Here it is measured: `tests/ runs 12 prompts against the real model and reports which attempt each one landed on, so the day routing starts eroding shows up as a number rather than a feeling. -The same idea runs through the rest. Every one of the 31 checks in the verification gate has a +The same idea runs through the rest. Every one of the 33 checks in the verification gate has a row in a suite that breaks what that check watches and requires the gate to go red naming it — a check nobody has watched fail is indistinguishable from a check that always passes. The installer is run into 22 throwaway home directories on Linux, macOS and Alpine on every @@ -72,9 +72,9 @@ cd vstack read the script before running it: ```bash -curl -fsSL https://raw.githubusercontent.com/itsvedantkumar/vstack/v1.8.0/bootstrap.sh -o bootstrap.sh +curl -fsSL https://raw.githubusercontent.com/itsvedantkumar/vstack/v1.9.1/bootstrap.sh -o bootstrap.sh less bootstrap.sh # about 100 lines -VSTACK_REF=v1.8.0 bash bootstrap.sh # installs that tag, not main +VSTACK_REF=v1.9.1 bash bootstrap.sh # installs that tag, not main ``` The unpinned one-liner is shorter and is what most people will paste: @@ -151,7 +151,7 @@ exits 127 on every hook event inside a sandbox. | Subagents | 8 | `~/.claude/agents/` | | Commands | 14 | `~/.claude/commands/` | | Hooks | 6 | `~/.claude/hooks/` | -| CLI wrappers | 7 | `~/.config/agents/bin/` | +| CLI wrappers | 6 | `~/.config/agents/bin/` | | MCP servers | 2 | merged into `~/.claude.json` | | Global directives | `CLAUDE.md` | `~/.claude/CLAUDE.md` | diff --git a/bin/doctor b/bin/doctor index 8fa3cc7..6b946e8 100755 --- a/bin/doctor +++ b/bin/doctor @@ -220,8 +220,17 @@ if [ "${1:-}" = "--drift" ]; then fi # Behind-remote is informational only — never fail solely for being behind. + # + # Every flag here is spelled out because a bare `git fetch` is not read-only: it does whatever + # the operator's ~/.gitconfig says. With fetch.prune and fetch.pruneTags set true -- a common + # pairing -- it deletes every local tag and remote-tracking branch that is not on the remote. + # That is exactly what happened during the 1.9.1 audit: `doctor --drift`, a command whose whole + # job is to compare and report, silently destroyed an unpushed release tag, and the release + # check then reported ok for a version whose tag had been deleted out from under it seconds + # earlier. An inspection command must not be able to change what it inspects, whatever the + # ambient config says. if git -C "$REPO" rev-parse --symbolic-full-name '@{u}' >/dev/null 2>&1; then - git -C "$REPO" fetch -q 2>/dev/null || true + git -C "$REPO" fetch -q --no-tags --no-prune --no-write-fetch-head 2>/dev/null || true behind=$(git -C "$REPO" rev-list --count 'HEAD..@{u}' 2>/dev/null || echo 0) case "$behind" in ''|*[!0-9]*) behind=0 ;; esac if [ "$behind" -gt 0 ]; then diff --git a/bin/doctor-cron.sh b/bin/doctor-cron.sh deleted file mode 100755 index b01117d..0000000 --- a/bin/doctor-cron.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/usr/bin/env bash -LOG="$HOME/.config/agents/doctor.log" -ts=$(date '+%Y-%m-%d %H:%M') -out=$("$HOME/.config/agents/bin/doctor" 2>&1); rc=$? -printf '\n=== %s (exit %s) ===\n%s\n' "$ts" "$rc" "$out" >> "$LOG" -if [ "$rc" -ne 0 ]; then - osascript -e 'display notification "Setup drift detected — run /doctor" with title "Agent Doctor" sound name "Basso"' >/dev/null 2>&1 || true -fi -exit 0 - diff --git a/bootstrap.sh b/bootstrap.sh index 97fd6d3..6d0d90f 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -103,5 +103,6 @@ if [ "$DEPS" = 1 ] && [ -x "$DIR/setup-machine.sh" ]; then echo fi -# shellcheck disable=SC2086 +# shellcheck disable=SC2086 # $ARGS is a caller-supplied flag list and has to word-split here; + # quoting it would hand install.sh one argument containing spaces exec "$DIR/install.sh" $ARGS diff --git a/claude/.claude-plugin/plugin.json b/claude/.claude-plugin/plugin.json index c4e066d..c4e0779 100644 --- a/claude/.claude-plugin/plugin.json +++ b/claude/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "vstack", - "version": "1.8.0", + "version": "1.9.1", "description": "Skills that fire on the situation instead of a slash command, plus the subagents, commands, and session hook that make them fire. Verification gates, parallel fan-out, code review, and writing discipline.", "author": { "name": "Vedant Kumar" diff --git a/setup-machine.sh b/setup-machine.sh index 825bd14..54e552c 100755 --- a/setup-machine.sh +++ b/setup-machine.sh @@ -311,6 +311,33 @@ else note "!! ${pl%@*} failed (add it later with: claude plugin install $pl)"; mark fail "${pl%@*}" fi done + + # claude-mem ships its UserPromptSubmit hooks synchronous, which puts its work on the critical + # path of every prompt. vstack wants them async, and bin/doctor has checked for that flag for + # several versions -- but nothing ever set it. This lane installs the plugin and then left the + # machine in a state its own doctor calls drift, so every fresh bootstrap ended on a red line + # telling the operator to "re-apply" something that had never been applied once. + # + # Idempotent by construction: it reads the flag first and rewrites only when it is not already + # set. That matters because claude-mem auto-updates rewrite hooks.json and revert it, which is + # the whole reason doctor watches the flag rather than trusting a one-time edit. + if command -v jq >/dev/null 2>&1; then + cm_cdir="${CLAUDE_CONFIG_DIR:-$HOME/.claude}" + for cm_f in "$cm_cdir"/plugins/cache/thedotmack/claude-mem/*/hooks/hooks.json; do + [ -f "$cm_f" ] || continue + jq -e '[.hooks.UserPromptSubmit[]?.hooks[]?.async] | all' "$cm_f" >/dev/null 2>&1 && continue + cm_t=$(mktemp) + if jq '.hooks.UserPromptSubmit = [ .hooks.UserPromptSubmit[]? + | .hooks = [ .hooks[]? | .async = true ] ]' "$cm_f" > "$cm_t" \ + && jq -e . "$cm_t" >/dev/null 2>&1; then + cat "$cm_t" > "$cm_f" + note ">> claude-mem UserPromptSubmit hooks set async" + else + note "!! could not set claude-mem hooks async in $cm_f" + fi + rm -f "$cm_t" + done + fi fi note "" diff --git a/tests/README.md b/tests/README.md index 6e4d81d..5e26d74 100644 --- a/tests/README.md +++ b/tests/README.md @@ -44,6 +44,16 @@ regression. specific skill to auto-fire, inspects the `stream-json` transcript for `Skill` tool_use blocks, and reports PASS/FAIL per case. +## evals/ + +`evals/run.sh`, `evals/run-pathways.sh` and `evals/swebench/run.sh` score this bundle against +other Claude Code setups and against unconfigured Claude Code. `evals/optimize.sh` drives the +change-one-thing-and-re-measure loop on top of `run-pathways.sh`; it scores against +`evals/holdout/` only through `--validate`, and never uses that set to decide whether to keep a +change. `evals/RESULTS.md` records every run, including the retracted one. + +These cost real model calls and are not part of the gate. + ## Run the tests ```bash diff --git a/tests/evals/RESULTS.md b/tests/evals/RESULTS.md index 9fd20e4..22bbf72 100644 --- a/tests/evals/RESULTS.md +++ b/tests/evals/RESULTS.md @@ -137,6 +137,12 @@ is a defect class this says nothing about. ## RETRACTED — Run of 2026-08-21 — 8 fixtures, 5 samples per arm (120 reviews) +> **The gstack row is worse than unfair, it is meaningless.** See "Four more ways this +> benchmark favoured its author" below: gstack's `/review` names helper scripts under +> `~/.claude/skills/gstack/` 85 times, the harness installs at project scope, and that directory +> has never existed on this machine. Every gstack number ever recorded here came from a pathway +> whose helpers were all `command not found`. +> > **These numbers are unfair to both harnesses and are retracted.** The scoring counted a > reviewer's nits, style notes and "no tests here" observations as false positives, while the > baseline had been explicitly instructed not to make such observations at all. So the precision @@ -234,3 +240,117 @@ only thing all the arms share.** Unresolved runs are now kept rather than deleted, because the moment you most need to look at what the agent did is the moment everything scored zero. + + +## Four more ways this benchmark favoured its author — 2026-08-22 + +Found by auditing the harness rather than running it. Each is stated with the command that +produced it. Three of the four make vstack look better than it is; the fourth makes it look +worse, and it is listed here for the same reason as the others. + +### 7. The gstack arm's helper scripts have never existed + +`gstack/review/SKILL.md` refers to `~/.claude/skills/gstack/...` 85 times, mostly executables +under `bin/`. `install_arm` copies gstack's skill directories into the fixture's *project* scope, +which does not create that path, and gstack has never been installed at user scope on this +machine. + +``` +$ git clone --depth 1 https://github.com/garrytan/gstack /tmp/gstack-fresh # 85fd9db +$ grep -c '\.claude/skills/gstack' /tmp/gstack-fresh/review/SKILL.md +85 +$ ls -d ~/.claude/skills/gstack +ls: no such file or directory +``` + +So every gstack review in every run recorded above executed a pathway whose every helper +invocation failed. The retracted n=5 table scored that wreckage at 68.6% recall and published it +as a comparison. + +`run-pathways.sh` now refuses an arm that declares paths which do not exist, before it sends a +single prompt: + +``` +$ GSTACK_DIR=/tmp/gstack-fresh tests/evals/run-pathways.sh --samples 1 --arms gstack +INVALID gstack: declares 114 path(s) that do not exist on this machine; not run, not scored +``` + +The same check reports nothing for `none` and `vstack`, so it is detecting a broken arm rather +than flagging everything. + +### 8. The gstack arm was handed a nested copy of the entire gstack repository + +`find "$GSTACK_DIR" -maxdepth 2 -name SKILL.md` was meant to find each skill's manifest. gstack +keeps a 34 KB `SKILL.md` at its repository root, which matches at depth 1, so `cp -R $(dirname +"$m")` copied the whole checkout — `.git`, every other skill, the lot — into +`.claude/skills/` as one skill. + +``` +$ find /tmp/gstack-fresh -maxdepth 2 -name SKILL.md | wc -l +54 +$ find /tmp/gstack-fresh -maxdepth 1 -name SKILL.md +/tmp/gstack-fresh/SKILL.md +``` + +Now `-mindepth 2 -maxdepth 2`. + +### 9. The validity gate could not fail + +`pathway_entered` read `.slash_commands[]` out of the session's `init` event. That field lists the +commands the session **registered**, not the ones it invoked, so it returned yes for every arm in +every run. + +``` +$ mkdir -p $D/.claude/commands && cat > $D/.claude/commands/probe.md # a command never invoked +$ cd $D && claude -p "What is 2+2? Answer with the number only." \ + --setting-sources=project --output-format=stream-json --verbose \ + | jq -rs '[.[]|select(.subtype=="init")|.slash_commands[]?]|map(select(.=="probe"))|length' +1 +``` + +A validity check that cannot fail is the same thing as no validity check. The replacement counts +three signals from the transcript, any one of which is engagement: a `Skill` or `Task` call, a +read under the arm's own `.claude` tree, or a `Bash` command running something out of it. The +first covers vstack's subagent style, the other two cover gstack's inline style, so it does not +privilege one architecture the way the version before last did. + +### 10. No vstack arm has ever included vstack's routing + +`install_arm` copied skills, commands and agents. It never copied `claude/hooks/` or applied the +project settings. `skill-mandate.sh` and `inject-session-context.sh` are the mechanism by which +vstack routes work to a skill; without them the arm is a directory of skill files with nothing +steering the model toward them. + +That is the exact configuration the first benchmark on this page already measured, where zero +`Skill` calls fired across sixty runs and no arm beat baseline. Every vstack number here is of +vstack-without-its-routing. The arm now gets the hooks and `overlay.sh`. + +This one cuts against vstack rather than for it. It is listed because the rule is that harness +defects get published, not that unflattering ones do. + +## Why there is no new number on this page + +The four defects above are fixed, and the harness is fairer than it has ever been. It was not +re-run, for two reasons, and neither is a result. + +A fair gstack arm cannot be built on this machine yet. Its pathway needs +`~/.claude/skills/gstack/`, which means either installing a competitor's toolchain into the real +home directory for the duration of a run, or rewriting its absolute paths to point at the project +copy. The first contaminates the `none` and `vstack` arms, which read user scope too. The second +edits the competitor. A per-arm scratch `HOME` would solve it, except that authentication here is +claude.ai OAuth held in the macOS keychain rather than a file, so a scratch home is an +unauthenticated one: + +``` +$ HOME=$(mktemp -d) claude -p "Say OK" --setting-sources=project +Not logged in · Please run /login +$ claude -p "Say OK" --setting-sources=project +OK +``` + +That is benchmark bug 1 in a new costume, and running the suite before solving it would publish a +fourth number produced by a broken gstack arm. + +The second reason is simpler: the run was not authorised. A six-arm, five-sample pass is roughly +480 model calls, and this account is a Max subscription rather than metered API billing, so the +cost is plan allowance rather than dollars. It is still not mine to spend without being asked. diff --git a/tests/evals/run-pathways.sh b/tests/evals/run-pathways.sh index 7320f67..6cc6cb1 100755 --- a/tests/evals/run-pathways.sh +++ b/tests/evals/run-pathways.sh @@ -110,19 +110,52 @@ install_arm() { # vstack) for x in "$SRC"/claude/skills/*/; do [ -d "$x" ] && cp -R "${x%/}" "$sk/"; done cp "$SRC"/claude/commands/*.md "$cm/" 2>/dev/null - mkdir -p "$d/.claude/agents" && cp "$SRC"/claude/agents/*.md "$d/.claude/agents/" 2>/dev/null ;; + mkdir -p "$d/.claude/agents" && cp "$SRC"/claude/agents/*.md "$d/.claude/agents/" 2>/dev/null + # Hooks and project settings, which no vstack arm had ever been given. skill-mandate.sh and + # inject-session-context.sh ARE vstack's routing: without them the arm is the skill files + # sitting on disk with nothing steering the model toward them, which is the configuration + # the very first benchmark already measured and found indistinguishable from baseline. + # Every published vstack number to date is of vstack-without-its-routing. + mkdir -p "$d/.claude/hooks" && cp "$SRC"/claude/hooks/*.sh "$d/.claude/hooks/" 2>/dev/null + chmod 755 "$d"/.claude/hooks/*.sh 2>/dev/null + [ -x "$SRC/overlay.sh" ] && "$SRC/overlay.sh" "$d" >/dev/null 2>&1 + ;; gstack) # The WHOLE skill directory, not just SKILL.md. gstack's /review reads sibling files — # specialists/, checklist.md, design-checklist.md — and copying only the manifest left its # pathway unable to run. It scored zero and was marked INVALID, which is the right outcome # for a broken arm and would have been a disgraceful thing to publish as a result. - find "$GSTACK_DIR" -maxdepth 2 -name SKILL.md 2>/dev/null | while IFS= read -r m; do + # + # -mindepth 2, not -maxdepth 2 alone. gstack keeps a 34 KB SKILL.md at its repo root, which + # matched at depth 1, so `cp -R $(dirname)` copied the entire checkout -- .git, every other + # skill, the lot -- into $sk/ as a single nested skill. Measured on 85fd9db: + # 54 matches at depth <= 2, one of them the repo itself. + find "$GSTACK_DIR" -mindepth 2 -maxdepth 2 -name SKILL.md 2>/dev/null | while IFS= read -r m; do d0=$(dirname "$m"); n=$(basename "$d0") cp -R "$d0" "$sk/$n" 2>/dev/null done ;; esac } +# Every path an arm declares as something it will execute or read, checked for existence before +# a single prompt is sent. +# +# This is the check that was missing. gstack's review pathway names ~/.claude/skills/gstack/bin/ +# helpers 85 times; that directory does not exist unless gstack was installed at user scope, and +# the harness installs at project scope. So every gstack run ever recorded here executed a +# pathway whose helpers were all `command not found`, scored the wreckage, and published it as a +# comparison. An arm that cannot run is INVALID. It is not a low score. +dangling_refs() { # -> one path per line, empty if the arm is sound + local d="$1" + [ -d "$d/.claude" ] || return 0 + grep -rhoE '(~|\$HOME)/\.claude/[A-Za-z0-9._/-]+' "$d/.claude" 2>/dev/null \ + | sort -u \ + | while IFS= read -r p; do + case "$p" in "~"*) real="$HOME${p#\~}" ;; '$HOME'*) real="$HOME${p#\$HOME}" ;; *) real="$p" ;; esac + [ -e "$real" ] || printf '%s\n' "$p" + done +} + run_arm() { # -> findings TAB pathway_entered local a="$1" d="$2" prompt raw text entered case "$a" in @@ -151,7 +184,27 @@ run_arm() { # -> findings TAB pathway_entered # marked INVALID for having a different — perfectly reasonable — implementation style. A # validity check that only recognises the author's own architecture is not a validity check, # it is a thumb on the scale. - entered=$(printf '%s' "$raw" | jq -rs '[.[]|select(.subtype=="init")|.slash_commands[]?]|map(select(.=="review"))|length' 2>/dev/null) + # + # And it is not "was the command registered" either, which is all the previous version + # measured. `.slash_commands` in the init event lists what the session REGISTERED, not what + # it invoked. Measured directly: a project with a /probe command that the prompt never + # mentions still reports probe in slash_commands, count 1. So this gate returned yes for + # every arm in every run and could not fail -- a validity check that cannot fail is the same + # thing as no validity check, which is the defect this whole file exists to avoid. + # + # Three signals, any one of which counts, and none of which privileges a particular + # architecture: a Skill or Task call naming something the arm installed, a file read under + # the arm's own .claude tree, or a Bash command that runs something out of it. The first + # covers vstack's subagent style, the second and third cover gstack's inline style. + entered=$(printf '%s' "$raw" | jq -rs --arg d "$d" ' + [ .[] | select(.type=="assistant") | .message.content[]? | select(.type=="tool_use") ] + | map( + if (.name=="Skill" or .name=="Task") then 1 + elif ((.name=="Read" or .name=="Grep" or .name=="Glob") + and ((.input.path // .input.file_path // .input.pattern // "") | contains($d + "/.claude"))) then 1 + elif (.name=="Bash" and ((.input.command // "") | contains($d + "/.claude"))) then 1 + else 0 end) + | add // 0' 2>/dev/null) [ "${entered:-0}" -gt 0 ] && entered=yes || entered=no fi # Extraction is a separate, identical pass for every arm. @@ -230,8 +283,26 @@ RUNLOG="${RUNLOG:-$ROOT/runs.tsv}" printf 'arm\tfixture\tsample\thits\tplanted\tfp\tentered\n' > "$RUNLOG" TOTAL_RUNS=$(( $(printf '%s' "$ARMS_CSV" | tr ',' ' ' | wc -w) * NFIX * SAMPLES )) DONE_RUNS=0 +DANGLING_REPORT="" for a in $(printf '%s' "$ARMS_CSV" | tr ',' ' '); do H=0; P=0; FP=0; ENT=0; RUNS=0 + + # Soundness before spend. Build the arm once, list every path it declares that does not exist, + # and refuse to run it at all if any do. Sixty gstack reviews were once bought and scored on an + # arm whose every helper was `command not found`; this costs one directory build and would have + # stopped that before the first prompt. + probe_dir="$ROOT/probe-$a" + make_repo "$(jq -r '.fixtures[0].file' "$GT")" "$probe_dir"; install_arm "$a" "$probe_dir" + dang=$(dangling_refs "$probe_dir") + if [ -n "$dang" ]; then + DANGLING_REPORT="$DANGLING_REPORT$a $(printf '%s' "$dang" | tr '\n' ' ') +" + printf 'INVALID %s: declares %s path(s) that do not exist on this machine; not run, not scored\n' \ + "$a" "$(printf '%s\n' "$dang" | grep -c .)" >&2 + printf '%s\n' "$dang" | sed 's/^/ /' >&2 + continue + fi + for f in $(jq -r '.fixtures[].file' "$GT"); do for s in $(seq 1 "$SAMPLES"); do d="$ROOT/$a-${f%.py}-$s" diff --git a/tests/gate-falsifiability.sh b/tests/gate-falsifiability.sh index e4e887a..b5072f2 100755 --- a/tests/gate-falsifiability.sh +++ b/tests/gate-falsifiability.sh @@ -18,7 +18,7 @@ set -uo pipefail cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 1 # One id per `# --- N.` section in .claude/verify.sh. Check 16 parses this line. -CHECKS="0 1 2 3 4 5 6 7 8 9 9b 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29" +CHECKS="0 1 2 3 4 5 6 7 8 9 9b 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31" BK=$(mktemp -d) NOJQ=$(mktemp -d) @@ -43,6 +43,22 @@ fail(){ printf 'FAIL check %-3s did NOT fail when broken\n expected label: save(){ for f in "$@"; do mkdir -p "$BK/$(dirname "$f")"; cp "$f" "$BK/$f"; done; } restore(){ for f in "$@"; do cp "$BK/$f" "$f"; done; } +# Rows whose mutation creates a file instead of editing one. save()/restore() work by copying, so +# a planted file has no backup to be put back from and has to be removed by name. Leaving it +# behind fails the tree-unchanged check at the end, which is correct, and then fails every run +# after this one too, which is not. +# +# The name is assembled from the PID rather than written out, for the same reason checks 4-6 +# assemble their secret probes: this file is a tracked file, so a literal basename here is a +# referrer as far as check 31 is concerned. The first draft spelled the name out, the check found +# it named in this very script, and the row reported "did NOT fail when broken" while the +# mutation was working perfectly. +ORPHAN_PROBE="bin/zz-unreferenced-$$.sh" + +creates_for(){ case "$1" in + 31) printf '%s' "$ORPHAN_PROBE" ;; +esac } + # Files each row edits, so it can be put back byte for byte. Backing up beats `git checkout` # here: this has to be safe to run on a dirty tree. files_for(){ case "$1" in @@ -72,7 +88,9 @@ files_for(){ case "$1" in 26) printf 'README.md' ;; 27) printf 'claude/hooks/skill-mandate.sh' ;; 28) printf 'README.md' ;; - 29) printf 'claude/hooks/format.sh' ;; + 29) printf 'bin/cloudflare-mcp' ;; + 30) printf 'claude/hooks/format.sh' ;; + 31) printf '' ;; # plants a new file rather than editing one esac } # The label the gate must print. Matched against the FAIL lines only. @@ -108,6 +126,8 @@ label_for(){ case "$1" in 27) printf 'skill mandate decides correctly' ;; 28) printf 'every doc is reachable' ;; 29) printf 'shellcheck clean' ;; + 30) printf 'shellcheck suppressions carry a reason' ;; + 31) printf 'every shipped file has a referrer' ;; esac } # Break exactly what the check watches, and nothing else. Surgical matters: a mutation that @@ -189,8 +209,23 @@ exit 0 && rm -f claude/.claude-plugin/plugin.json.t ;; 29) # An unquoted expansion, which is the single most common way a shell script breaks on # somebody else's machine: a path with a space in it silently becomes two arguments. + # + # Broken in bin/cloudflare-mcp on purpose. It is a #!/bin/sh script with no .sh suffix, so + # the old `git ls-files '*.sh' bin/doctor bin/vstack` selector never linted it and this + # exact mutation left the check green. Mutating a file the selector already covered would + # prove the linter runs; mutating this one proves it runs over everything. printf '\nsc_probe=$HOME/some path\nls $sc_probe >/dev/null 2>&1 || true\n' \ + >> bin/cloudflare-mcp ;; + 30) # A bare disable, no reason on the line and none above it. This is the shape bootstrap.sh + # carried for several versions while check 29's own header claimed the rule was kept. + printf '\n# shellcheck disable=SC2086\nsup_probe=$HOME/x\nls $sup_probe >/dev/null 2>&1 || true\n' \ >> claude/hooks/format.sh ;; + 31) # A file nothing points at. It has to be tracked to be visible to the check, so it is added + # to the index and removed again after the row -- the same shape as every other row, except + # the mutation creates rather than edits, so creates_for() cleans up instead of restore(). + printf '#!/usr/bin/env bash\necho probe\n' > "$ORPHAN_PROBE" + chmod +x "$ORPHAN_PROBE" + git add -f "$ORPHAN_PROBE" >/dev/null 2>&1 ;; 28) # Strand a document by removing the only link to it, which is how a 783-line research # handoff came to sit in docs/ reachable from nothing. perl -0pi -e 's{- \[Provenance\]\(docs/provenance/README\.md\)[^\n]*\n}{}' README.md ;; @@ -219,7 +254,10 @@ echo # break back, and the suite printed FALSIFIABLE over a repo that still carried the defect. The # tree-unchanged check at the end cannot see that -- it compares the run against a start that was # already wrong. Only a baseline can. -if ! base=$(./.claude/verify.sh 2>&1) || printf '%s' "$base" | grep -q '^FAIL '; then +# Captured to a variable and grepped from a here-string, never `printf ... | grep -q`. Under +# `set -o pipefail` grep -q exits the moment it matches, the writer upstream takes SIGPIPE, and +# the pipeline reports 141 -- which reads as "no FAIL found" and declares a red baseline green. +if ! base=$(./.claude/verify.sh 2>&1) || grep -q '^FAIL ' <<<"$base"; then printf 'FAIL gate is not green before any mutation; nothing here would be evidence:\n%s\n' \ "$(printf '%s' "$base" | grep -E '^(FAIL|VERIFICATION)' | sed 's/^/ /')" printf '\n0 passed, 1 failed\nNOT FALSIFIABLE\n' @@ -245,7 +283,11 @@ for id in $CHECKS; do # assuming. Without tags there is nothing for it to compare, and demanding a FAIL it cannot # produce turns a correct skip into a red build. if [ "$id" = 24 ]; then - if ./.claude/verify.sh 2>&1 | grep -q "^skip $lbl"; then + # Not `verify.sh | grep -q`: verify writes for ~20s, grep -q exits on the first match, verify + # dies of SIGPIPE, and pipefail turns the whole pipeline into 141. That read as "the check did + # not skip", so this branch never fired and the row claimed a falsifiability it had not shown. + _probe=$(./.claude/verify.sh 2>&1) + if grep -q "^skip $lbl" <<<"$_probe"; then printf 'skip check %-3s not falsifiable here (no tags to compare against; %s)\n' "$id" "$lbl" continue fi @@ -256,7 +298,8 @@ for id in $CHECKS; do printf 'skip check %-3s not falsifiable here (claude CLI not installed; %s)\n' "$id" "$lbl" continue fi - if ./.claude/verify.sh 2>&1 | grep -q "^skip $lbl"; then + _probe=$(./.claude/verify.sh 2>&1) # not a pipe: see the 141 note on check 24 above + if grep -q "^skip $lbl" <<<"$_probe"; then printf 'skip check %-3s not falsifiable here (validator is not validating; %s)\n' "$id" "$lbl" continue fi @@ -272,13 +315,18 @@ for id in $CHECKS; do out=$(./.claude/verify.sh 2>&1) fi - if printf '%s' "$out" | grep -q '^FAIL '"$lbl"; then + if grep -q '^FAIL '"$lbl" <<<"$out"; then pass "$id" "$lbl" else fail "$id" "$lbl" "$(printf '%s' "$out" | grep -E '^(FAIL|VERIF)' | sed 's/^/ got: /')" fi [ -n "$fs" ] && restore $fs + cr=$(creates_for "$id") + if [ -n "$cr" ]; then + git rm -q -f --cached "$cr" >/dev/null 2>&1 + rm -f "$cr" + fi done echo diff --git a/tests/install-matrix.sh b/tests/install-matrix.sh index d122a5f..e0c4163 100755 --- a/tests/install-matrix.sh +++ b/tests/install-matrix.sh @@ -658,10 +658,23 @@ if want uninstall-clean; then # things that are theirs, which must all still be here afterwards printf '{"theirKey":"keep","theme":"dracula","skillOverrides":{"their-skill":"off"}}\n' > "$H/.claude/settings.json" printf 'THEIR_MANAGED=true\n' > "$H/.conductor/settings.managed.toml" + printf '{"mcpServers":{"their-server":{"command":"theirs"}}}\n' > "$H/.claude.json" HOME="$H" "$SRC/install.sh" >/dev/null 2>&1 + # Positive control, taken between install and uninstall. Asserting only that vstack's servers + # are gone afterwards passes for free on any machine where they were never registered, which + # is the shape of every fake green this repo has shipped. + installed_srv=0 + command -v jq >/dev/null 2>&1 \ + && installed_srv=$(jq -r '[(.mcpServers // {}) | keys[]] | length' "$H/.claude.json" 2>/dev/null || echo 0) + installed_cond=0 + [ -f "$H/.conductor/settings.toml" ] && installed_cond=1 HOME="$H" "$SRC/uninstall.sh" --yes >/dev/null 2>&1 e="" + [ "${installed_cond:-0}" -eq 1 ] || e="$e; install never wrote ~/.conductor/settings.toml, so its removal proves nothing" if command -v jq >/dev/null 2>&1; then + want_srv=$(jq -r 'keys | length' "$SRC/mcp/servers.json" 2>/dev/null || echo 0) + [ "${installed_srv:-0}" -gt "$want_srv" ] \ + || e="$e; install registered ${installed_srv:-0} MCP servers where theirs plus $want_srv were expected, so the removal assertions below prove nothing" n=$(jq -r '[.hooks[]?[]?.hooks[]?.command]|length' "$H/.claude/settings.json" 2>/dev/null || echo 0) [ "${n:-0}" -eq 0 ] || e="$e; $n hook commands left pointing at deleted scripts" [ "$(jq -r '.model // "gone"' "$H/.claude/settings.json")" = gone ] || e="$e; vstack model policy still in force" @@ -672,6 +685,20 @@ if want uninstall-clean; then fi grep -q THEIR_MANAGED "$H/.conductor/settings.managed.toml" 2>/dev/null \ || e="$e; their Conductor managed policy was not restored" + # install.sh writes ~/.conductor/settings.toml where none exists. uninstall.sh had no + # reference to conductor at all, so an install into a clean home left both files behind for + # good -- and the managed one is the pinning file, so a removed vstack went on pinning models. + [ -f "$H/.conductor/settings.toml" ] && e="$e; ~/.conductor/settings.toml was left behind" + # Same shape one file over: install.sh merges its servers into the global mcpServers map, and + # nothing subtracted them again. Theirs must survive, ours must not. + if command -v jq >/dev/null 2>&1; then + for srv in $(jq -r 'keys[]' "$SRC/mcp/servers.json" 2>/dev/null); do + [ "$(jq -r --arg s "$srv" 'if (.mcpServers // {}) | has($s) then "left" else "gone" end' "$H/.claude.json" 2>/dev/null)" = gone ] \ + || e="$e; vstack's $srv MCP server was left registered" + done + [ "$(jq -r '(.mcpServers // {})["their-server"].command // "GONE"' "$H/.claude.json" 2>/dev/null)" = theirs ] \ + || e="$e; their own MCP server was removed" + fi [ -f "$H/.config/agents/verify-trust" ] && e="$e; the trust store was left behind" for rc in .zshrc .zshenv .bashrc; do [ -f "$H/$rc" ] && grep -q 'claude-parity' "$H/$rc" 2>/dev/null && e="$e; $rc still sources vstack" @@ -680,6 +707,60 @@ if want uninstall-clean; then || bad "uninstall removes vstack and keeps your own settings" "${e#; }" fi +# --- doctor --drift does not mutate the repo it inspects --------------------------------------- +# A bare `git fetch` is not read-only: it does whatever ~/.gitconfig says. With fetch.prune and +# fetch.pruneTags true -- a common pairing -- it deletes every local tag and remote-tracking +# branch the remote does not have. doctor --drift ran one, and during the 1.9.1 audit it silently +# destroyed an unpushed release tag, after which the release check reported ok for a version +# whose tag was already gone. Ambient config must not be able to turn an inspection into an edit. +if want doctor-no-mutate; then + if ! command -v git >/dev/null 2>&1; then + skip "doctor --drift leaves the repo alone" "git not installed" + else + T="$ROOT/nomutate"; mkdir -p "$T" + # A real vstack checkout, because --drift refuses to run against anything else and a scratch + # repo made it bail before ever reaching the fetch -- which is how the first version of this + # case passed against the unfixed doctor. + # A copy of the tree with a fresh history, not a clone. --drift only requires that the + # directory look like a vstack checkout, and cloning inherited the source repo's shape: CI + # checks out shallow, a clone of a shallow repo is shallow, and pushing one to a bare remote + # is rejected outright with "shallow update not allowed". One commit is enough for + # everything --drift reads. + mkdir -p "$T/work" + cp -R "$SRC"/. "$T/work"/ 2>/dev/null + rm -rf "$T/work/.git" + git -C "$T/work" init -q + git -C "$T/work" config user.email t@example.com; git -C "$T/work" config user.name t + git -C "$T/work" add -A >/dev/null 2>&1 + git -C "$T/work" commit -qm probe >/dev/null 2>&1 + git -C "$T/work" checkout -q -B probe-main + git init -q --bare "$T/remote.git" + git -C "$T/work" remote add origin "$T/remote.git" + # the destructive pairing, set locally so the case does not depend on the operator's config + git -C "$T/work" config fetch.prune true + git -C "$T/work" config fetch.pruneTags true + # push -u in one step. Setting the upstream separately depended on the push having created + # refs/remotes/origin/probe-main, which it did not do on the CI runners, and the case then + # failed on its own control with no way to see why from the log. + git -C "$T/work" push -u origin probe-main > "$T/push.log" 2>&1 || true + git -C "$T/work" tag -a v9.9.9-local -m "never pushed" 2>/dev/null + e="" + # Two positive controls. Without them the case passes on any machine where the tag was never + # created or where --drift declined to run, which is the shape of every fake green here. + git -C "$T/work" rev-parse -q --verify refs/tags/v9.9.9-local >/dev/null 2>&1 \ + || e="$e; the probe tag was never created, so this case proves nothing" + git -C "$T/work" rev-parse --symbolic-full-name '@{u}' >/dev/null 2>&1 \ + || e="$e; no upstream, so the code path under test never runs [$(tr '\n' ' ' < "$T/push.log" 2>/dev/null | cut -c1-160)]" + HOME="$T" VSTACK_DIR="$T/work" "$SRC/bin/doctor" --drift > "$T/out" 2>&1 + grep -q 'no vstack repo found' "$T/out" 2>/dev/null \ + && e="$e; --drift refused to run, so it never reached the fetch this case is about" + git -C "$T/work" rev-parse -q --verify refs/tags/v9.9.9-local >/dev/null 2>&1 \ + || e="$e; doctor --drift deleted an unpushed local tag" + [ -z "$e" ] && ok "doctor --drift leaves the repo alone" \ + || bad "doctor --drift leaves the repo alone" "${e#; }" + fi +fi + # --- overlay does not delete settings the target repo owns ------------------------------------- # It used to delete every key vstack ships that is not on the project allowlist, on the theory # that it was cleaning up its own past overlays. It cannot know that. A repository that diff --git a/uninstall.sh b/uninstall.sh index c25073c..15d64b8 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -159,7 +159,11 @@ done # settings.json and .claude.json are deliberately not in this list. install.sh MERGES those # rather than copying them, so there is no version of them that belongs solely to vstack — # deleting either would take the user's own configuration with it. If the backup holds a prior -# copy, the restore pass above already put it back. +# copy, the restore pass above already put it back, and for .claude.json it always does: +# install.sh backs the file up before merging, and creates it as `{}` first where it is absent, +# so there is always a prior copy to come back to. The uninstall-clean matrix case asserts that +# end to end -- vstack's mcpServers entries gone, the user's own still registered -- with a +# positive control that the entries were there to begin with. # # A file only qualifies if it is still byte-identical to the copy vstack installed. Once it # differs, somebody edited it, and their edit is not this script's to throw away — CLAUDE.md is @@ -198,6 +202,14 @@ for f in "$SRC"/claude/commands/*.md; do [ -e "$f" ] && plan_file_removal "$CDIR for f in "$SRC"/bin/*; do [ -e "$f" ] && plan_file_removal "$HOME/.config/agents/bin/$(basename "$f")" "$f"; done plan_file_removal "$CDIR/CLAUDE.md" "$SRC/claude/CLAUDE.md" plan_file_removal "$CDIR/statusline.sh" "$SRC/claude/statusline.sh" +# Conductor. install.sh writes settings.toml where none exists and rewrites settings.managed.toml +# every time, and this script had no reference to conductor at all -- so an uninstall left both in +# place permanently. The managed file is the one that pins models, fast mode and plan mode, which +# means a removed vstack went on setting a machine's policy. The same three rules apply as to +# everything else here: a backup wins, an edited file is kept and named, and only a file still +# byte-identical to what vstack shipped is removed. +plan_file_removal "$HOME/.conductor/settings.toml" "$SRC/conductor/settings.toml" +plan_file_removal "$HOME/.conductor/settings.managed.toml" "$SRC/conductor/settings.managed.toml" plan_file_removal "$CDIR/skills/LICENSE.pstack" "$SRC/claude/skills/LICENSE.pstack" plan_file_removal "$CDIR/skills/ATTRIBUTION.md" "$SRC/claude/skills/ATTRIBUTION.md" plan_file_removal "$HOME/.config/agents/shell/claude-parity.zsh" "$SRC/shell/claude-parity.zsh"