Audit: nine greens that measured nothing, and the 141 that hid them - #2
Merged
Conversation
An untagged-but-declared version has no payload to diff, so that branch compared nothing and said ok. That is the same defect the tagless branch one elif below already guards against, and printing ok hid it from the skip census. It now skips with a reason, so 31 declared / 30 ran / 1 skipped is what the gate reports until the release unit tags. Fixing that surfaced a second one. tests/gate-falsifiability.sh probed for the skip with `./.claude/verify.sh | grep -q`, and under set -o pipefail grep -q exits on the first match, verify dies of SIGPIPE, and the pipeline returns 141. That reads as "did not skip", so the branch never fired and row 24 reported falsifiable off the docs-pin mutation instead. Measured: rc=141 with pipefail, rc=0 without. All four verify-into-grep -q sites now capture first and grep a here-string. The suite now says 32 passed with row 24 honestly skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Check 29 selected its files with `git ls-files '*.sh' bin/doctor bin/vstack`, a list somebody has to remember to update. bin/cloudflare-mcp is a #!/bin/sh script with no .sh suffix and had never been on it: appending an unquoted $HOME/some path to that file made shellcheck exit 1 while the gate still printed "ok shellcheck clean (29 scripts)". Selection is now by shebang, the way check 1 already did it, and the count moved 29 -> 31. Falsifiability row 29 now mutates bin/cloudflare-mcp rather than a file the old selector already covered, so it proves the linter runs over everything instead of that it runs. Check 12 had the same shape one level up. want_for() resolved eight nouns and the extractor carried its own separate grep alternation, so a claim could be extractable-but-unresolvable or the reverse, silently either way. Both now come from one NOUNS list, with a positive control that fails if the extractor looks for a noun want_for cannot resolve. Added "shell scripts", which surfaced CHANGELOG's "29 shell scripts" against a tree of 31, and added CHANGELOG's Unreleased and current-version sections to the scan. Older entries stay out: they record what was true then, and editing them to satisfy today's tree would be falsifying history. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Check 29's header has claimed for several versions that a shellcheck suppression carries its reason with it. Nothing checked, and bootstrap.sh:106 had carried a naked disable=SC2086 since that lane was written. That is the second rule here to live in prose and be skipped by whoever did not read the prose, so it becomes check 30 rather than another sentence. A reason counts on the same line after the code list, which is how the other five are written, or on the line immediately above. The selector anchors on a comment-leading directive, which is the only form shellcheck honours anyway; matching the bare phrase also caught this file's own prose about the rule and reported 9 suppressions where there are 6. Watched red on bootstrap.sh:106 and again on a planted bare disable in format.sh before going green. Row 30 in the falsifiability suite plants that same shape. README moves 31 -> 32 checks in this commit because check 12 goes red between adding the check and updating the claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Check 28 already required every doc to be reachable. Nothing said the same about the rest of the tree, 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 the eval-loop driver, which nothing but its own header mentioned. The wrapper is deleted. Cloud routines are the scheduling lane here, so a launchd shim is not an unfinished feature, it is a leftover. The eval driver gets a real referrer: tests/README.md now has an evals/ section that says what each harness does and that the holdout set is only ever scored through --validate. bin/* drops 7 -> 6, so the README table moves in this commit or check 12 goes red between the two edits. Check 31 defeated itself twice before it worked, and both times the same way. Naming the orphans in the check's own header gave them a referrer; naming the probe file in the falsifiability row gave the probe one, and the row reported "did NOT fail when broken" while the mutation was working. The header now describes the two files without naming them, the probe takes its name from $$, and both comments say why. The limit is stated rather than hidden: a mention in prose counts as a referrer, so this finds files nothing points at, not files pointed at only rhetorically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
install.sh writes ~/.conductor/settings.toml where none exists and
rewrites settings.managed.toml on every run. uninstall.sh contained no
reference to conductor at all, so both survived an uninstall for good.
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.
Both now go through plan_file_removal, so the existing three rules
apply unchanged: a backup wins, an edited file is kept and named, and
only a file still byte-identical to what vstack shipped is removed.
The uninstall-clean case grew the assertion, watched red first. It also
grew two positive controls, because the obvious way to write this test
passes for free: asserting only that something is gone afterwards proves
nothing on a machine where it was never installed, which is the shape of
every fake green this repo has shipped. It now checks that install put
the conductor file and the MCP servers there before checking they left.
That control settled the .claude.json question too. Its mcpServers
entries were already being unpicked correctly by the backup-restore
pass, which always has a prior copy to work from because install.sh
creates the file as {} before merging. Measured: three servers after
install, the user's one after uninstall. The assertion stays as a
regression guard and the header note now says so instead of implying
the case is unhandled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README's pin-a-release lane pinned v1.8.0, the manifests said v1.8.0, and no such tag existed. Check 24 compared the two strings, found them equal, and was satisfied. Measured with curl: HTTP 404 for v1.8.0, 200 for v1.7.0. Every stranger who copy-pasted that block got nothing, and the check written to stop exactly that could not see it because it was only ever comparing docs against manifest, never against the repository. A pin now has to name a tag that is actually there, asserted only where the checkout has tags at all so a shallow clone still declines to measure. CI's gate job uses fetch-depth 0, so it runs there. Manifests and README move to 1.9.0 and the CHANGELOG entry describing this audit lands in the same commit. The tag follows immediately, which is what turns check 24 from skipping back to measuring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
setup-machine.sh installs claude-mem. bin/doctor has checked that the plugin's UserPromptSubmit hooks are async for several versions. Nothing ever set the flag, so every fresh bootstrap ended on a red line telling the operator to re-apply something that had never been applied once, and the whole lane closed with DRIFT. The install-matrix doctor-stranger case never saw this: it exercises install.sh, and the plugin only arrives via setup-machine.sh, which only the bootstrap lane runs. Found by running the README quickstart verbatim into a scratch HOME, which is the point of running it verbatim. Idempotent by construction -- it reads the flag and rewrites only when it is not already set. Measured on a scratch HOME: first run reports setting it, second run reports nothing, and after reverting the flag by hand the way a claude-mem auto-update does, the next run sets it again. Doctor on that HOME goes from one red line and DRIFT to 23 ok, 0 red, 6 notes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Check 24 named setup-machine.sh as payload that moved after the v1.9.0 tag, which is what it is for. Bumping rather than moving the tag: v1.9.0 is published and describes a payload carrying the claude-mem defect, and a tag somebody may have fetched is not a thing to rewrite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--drift ran a bare `git fetch` in the vstack checkout to compute how far behind the remote it was. A bare fetch is not read-only: it does whatever the operator's ~/.gitconfig says, and with fetch.prune and fetch.pruneTags set true -- a common pairing, and the one on this machine -- it deletes every local tag and remote-tracking branch the remote does not have. It destroyed the v1.9.1 tag seconds after it was created. Check 24 had already printed "ok (v1.9.1)" against the tag that existed at the time, and by the time the push ran there was no such ref. The symptom looked like a fake green in the release check; the cause was an inspection command mutating what it inspects. Every flag is now explicit: --no-tags --no-prune --no-write-fetch-head. The behind-count still works and ambient config cannot change what the command does. The doctor-no-mutate case clones a real vstack checkout, sets that config pairing locally, plants an unpushed tag and asserts it survives. Three controls, because the first version of the case used a scratch repo that --drift refused to run against, so it never reached the fetch and passed against the unfixed doctor. Watched red on the real bug before green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by auditing the harness rather than running it. gstack's /review names helper scripts under ~/.claude/skills/gstack 85 times. install_arm installs at project scope, which does not create that path, and gstack has never been installed at user scope here. Every gstack review ever recorded on this machine executed a pathway whose every helper was command not found, and the retracted n=5 table scored that wreckage at 68.6% recall. run-pathways.sh now refuses an arm that declares paths which do not exist -- 114 of them for gstack -- before it sends a single prompt, and reports nothing for none or vstack. find -maxdepth 2 -name SKILL.md matched gstack's 34 KB root SKILL.md at depth 1, so cp -R copied the entire checkout, .git and all, in as a single nested skill. Now -mindepth 2. The validity gate read .slash_commands from the init event, which lists what the session REGISTERED, not what it invoked. Measured: a /probe command the prompt never mentions still reports count 1. It returned yes for every arm in every run and could not fail. Replaced with three signals from the transcript -- a Skill/Task call, a read under the arm's own .claude tree, or a Bash command running out of it -- so it covers gstack's inline style without privileging vstack's subagent style. And the one that cuts the other way: no vstack arm has ever been given claude/hooks or the project settings. skill-mandate.sh and inject-session-context.sh ARE vstack's routing. Every vstack number on that page is of vstack-without-its-routing, which is the configuration the first benchmark already measured and found no better than baseline. The doctor-no-mutate case is hardened for CI, where the checkout is a detached PR merge ref and --set-upstream-to had no branch to attach to. Its positive control caught that as a failure instead of a silent pass, which is what it is there for. No new number is published. RESULTS.md says why: a fair gstack arm needs a per-arm HOME, authentication here is keychain OAuth so a scratch HOME is an unauthenticated one, and the run was not authorised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Setting the upstream separately depended on the push having created refs/remotes/origin/probe-main, which it did not on the CI runners. The case then failed on its own positive control with nothing in the log explaining it, so the fix was guesswork twice. push -u does both, and the control now carries the push output when it fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI checks out shallow for the install jobs, a clone of a shallow repo is shallow, and pushing one to a bare remote is rejected outright with "shallow update not allowed". The case then failed on its own positive control, correctly, three times running. --drift only needs the directory to look like a vstack checkout, so the probe is now a copy of the tree with a one-commit history of its own. Nothing it reads needs more than that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
itsvedantkumar
added a commit
that referenced
this pull request
Aug 26, 2026
Rick's second review: stale() conflated two states behind one green-vs-STALE axis -- a consumer that exits 0 and never mentions the plant (genuinely structurally blind) versus a consumer that exits nonzero without mentioning it (failed for an unattributed reason, not proven blind). Add unknown() and rewire the three call sites that captured an rc and didn't gate on it: plugin-manifests.sh, install-matrix.sh default (dropped the "\|FAIL" OR fallback that let any unrelated failure read as noticed), and bin/doctor --drift's mcp_servers branch. Root cause of run #2's six plugin-manifests.sh exit=1 findings, confirmed by isolated reproduction (mkdir -p claude/skills/vstack-fixture-skill against a clean tree reproduces the exact failure signature): do_unplant() restored the planted file via cg_restore but never removed the directory do_plant() had mkdir -p'd around it. The empty leftover directory was invisible to `git status --porcelain` (which was the run's own tree-clean invariant) but visible to plugin-manifests.sh's filesystem glob, so each family's plant contaminated the next family's supposedly-clean baseline. Fixed by rmdir-ing the plant's parent directory (no-op when non-empty, which covers every family but skills) after cg_restore. Documented in do_unplant() with the run #2 evidence and reproduction steps inline. Rick's specific quote of lines 391-395 ("$pm_rc captured, not in the condition") did not match the committed code at 36f4580 -- verified via `git show 36f4580:tests/inventory-fixture.sh`, which already gated on `[ "$pm_rc" -ne 0 ] && grep -q "$name"`. The underlying concern (STALE vs. UNKNOWN conflation) was real and is what this commit fixes; the specific line citation was not. Re-run once, in a fresh worktree anchored to 16c734d, with this exact file (byte-identical, confirmed via diff against the working tree before commit): /tmp/vstack-invfixture-final-1787763997, log at /tmp/invfix-final.log Result: 7 ok, 0 FAIL, 18 STALE CONSUMER finding(s), 0 UNKNOWN finding(s) All 18 STALE findings resolve on exit=0 (structural blindness); zero resolved via the new UNKNOWN branch, confirming the six exit=1s in run #2 were entirely the directory-leak artifact above, not consumer signal. Positive controls (before and after the full matrix) unchanged: 0 FAIL, tree porcelain-identical at start and end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nine defects, every one of them a green that measured nothing or a command doing something its name does not suggest. Each leaves behind a check and a mutation row rather than just a fix.
What was broken
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 oneelifbelow already knew better. Now it skips with a reason, so the skip census can see it.The pinned quickstart 404s. The README's pin-a-release lane pinned v1.8.0, the manifests said v1.8.0, and no such tag existed. Check 24 compared the two strings, found them equal, and was satisfied. Measured:
HTTP 404for v1.8.0,200for v1.7.0. Every stranger who copy-pasted that block got nothing. 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/vstacknever includedbin/cloudflare-mcp, a#!/bin/shscript with no.shsuffix. An unquoted expansion appended to it made shellcheck exit 1 while the gate printedok shellcheck clean (29 scripts). Selection is by shebang now, the way check 1 already did it. Row 29 mutates that file specifically.The count check dropped nouns on the floor.
want_for()resolved eight nouns; the extractor carried its own grep alternation. A claim could be extractable-but-unresolvable or the reverse, silently either way. One list drives both now, with a positive control.A suppression-reason rule that only lived in a comment. Check 29's header had claimed it for versions while
bootstrap.shcarried a nakeddisable=SC2086. Check 30 enforces it.Two files nothing pointed at. A launchd wrapper around the doctor that
install.shnever installed anduninstall.shnever removed, and the eval-loop driver. Check 31 makes an unreferenced file a failure.An uninstall that left Conductor pinning policy.
install.shwrites~/.conductor/settings.tomlandsettings.managed.toml;uninstall.shhad no reference to conductor at all, so a removed vstack went on pinning models and plan mode.A fresh bootstrap ended red.
setup-machine.shinstalls claude-mem,bin/doctorhas checked its hooks are async for versions, and nothing ever set the flag. Thedoctor-strangermatrix case could not see it: that case runsinstall.sh, and the plugin only arrives viasetup-machine.sh, which only the bootstrap lane runs. Found by running the README quickstart verbatim, which is the point of running it verbatim.doctor --driftdeleted an unpushed release tag. It ran a baregit fetchto compute a behind-count. A bare fetch is not read-only: withfetch.pruneandfetch.pruneTagstrue it deletes every local tag the remote lacks. It destroyed the v1.9.1 tag seconds after creation, and the release check then reportedokfor a version whose tag was already gone. Every flag is explicit now.The 141 that hid several of these
tests/gate-falsifiability.shprobed for a check's skip withverify.sh | grep -q. Underset -o pipefail,grep -qexits on the first match,verifydies of SIGPIPE, and the pipeline returns 141, which reads as "did not skip". Measured:rc=141with pipefail,rc=0without. Row 24 had been reporting falsifiable off the wrong branch. Four sites now capture first and grep a here-string.Numbers
.claude/verify.shtests/gate-falsifiability.shtests/install-matrix.shtests/auto-trigger.shtests/compare-baseline.shDRIFT ✖on the bootstrap laneHTTP 404HTTP 200, 26/8/14/6/6 land, 0 redTwo checks that defeated themselves first
Naming the orphans in the header of the check that hunts unnamed files gave them a referrer. Naming the probe in the mutation row gave the probe one, and the row reported "did NOT fail when broken" while the mutation was working perfectly. Both comments now say so. The first version of
doctor-no-mutateused a scratch repo--driftrefuses to run against, so it never reached the fetch and passed against the unfixed doctor; it carries three controls now.What a reader should be sceptical of
bin/doctor --driftwas attacked with an empty source directory on the theory that it would printno drift ✔over nothing. It went red instead, correctly. It still does not say how many items it compared, which is a transparency gap rather than a defect.The
grep -q141 trap did not reproduce in isolation on this bash, only in the real slow-producer case inside the falsifiability suite. The other pipe-fedgrep -qsites are documented but unproven.No dead functions or unread variables were found. The five candidates the sweep surfaced were all false positives: two secret-scan test fixtures, an
IFS=read-loop idiom, and two env-prefix assignments.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.
Merge with a merge commit, not a squash:
v1.9.0andv1.9.1point at commits on this branch.🤖 Generated with Claude Code