improvise/context/test-evidence/tooling.md

# Test evidence — the repository's own gates

Evidence for the checks in `scripts/` and for the tests that hold CI's shape.
The arguments behind the checkers themselves are in `context/code/tooling.md`;
this file records what fault each test was watched to catch.

Note what is deliberately absent. `scripts/check-comment-runs.py --self-test`,
`scripts/kb-test.sh` and `scripts/assemble-stamp-test.sh` are check-the-checker
suites whose cases are already injected faults, and none of their case names is
a Rust `fn` or a JS `test(` the evidence checker could resolve — so their
evidence is the case list itself plus `context/code/tooling.md`, and inventing
headings for them here would be headings naming nothing.

### "no test in web/tests/ reaches for a built bundle"

The `js` CI job runs `node --test web/tests/*.test.js` and installs nothing; the
`browser` job runs `web/tests/bundle/*.test.js` after three wasm-pack builds.
The moment a flat `web/tests/` file reads `web/pkg/`, the fast job inherits the
slow one's dependency — and the likely repair is to add a build step to it,
silently spending the ~4m30s the split exists to avoid. Until improvise-4ktt
that constraint was a directory convention and a glob, checked by nothing, and
it failed in the direction that looks like ordinary breakage: a confusing ENOENT
in CI on a machine where the bundle happens not to be built.

Fault injected, 2026-09-21: appended
`const injected = readFileSync("../pkg/improvise_wasm_client.js");` to
`web/tests/grid_plan.test.js`. The test failed naming that file and pointing at
`web/tests/bundle/`. Reverted; green again.

Two things the test does that a simpler one would not. It asserts its own
filename is among the files it scans, so it cannot become the one place a
violation may hide. And it strips comments before matching, because this file's
own header and the test's header both name the directory in prose — a lexical
grep over raw text would have had to exempt them, and an exemption is where the
next violation goes.

**What it cannot see**, stated because the check itself uses the evasion: a path
assembled at runtime (`"../" + "p" + "kg"`) is invisible to a lexical match, and
the test's own `FORBIDDEN` constant is built exactly that way to stay out of its
own scan. That is a deliberate, disclosed hole — closing it needs a parser, and
the failure it would catch is one somebody has to write on purpose, where this
gate exists for the one written by accident.

### "the subdirectory that MAY use a bundle is not the one being checked"

The companion, and the one that stops the first from passing vacuously. If
`web/tests/bundle/` were emptied or its files stopped touching the bundle, the
first test would still be green while having nothing to be scoped *against* —
"no flat test uses the bundle" is uninteresting if no test anywhere does. This
asserts the other half of the split exists and really does what the flat half
is forbidden to do.

Fault injected, 2026-09-21: moved `web/tests/bundle/wasm_surface.test.js` out of
the tree. The test failed with "web/tests/bundle/ is empty — the split lost its
other half" rather than passing. Restored; green again.

### "no test in web/tests/ imports a module that reaches the bundle"

The companion to the lexical check above, and the reason that one is not
enough: `import "../app.js"` needs the bundle *transitively* and names nothing
forbidden, so the `js` job breaks loudly but unattributed. `app.js` and
`worker.js` are the two entry points that load the bundle; `shell.js` and
`serial.js` do not, and `serial.test.js` imports one legitimately.

Fault injected, 2026-09-21: appended `import "../app.js";` to
`web/tests/grid_plan.test.js`. The lexical test stayed green; this one failed
naming the file. Reverted.

### Note — the checker's own blockers, found by review of PR #83

Two gates in `scripts/check-comment-runs.py` produced a pass while not doing
what their prose said. Both were in the mode the same PR rewrote, which is the
part worth keeping: writing the doctrine down does not exempt the writer.

**A leaf name is not an identity.** Six leaf names collide in this workspace and
two were cited in evidence. Renaming
`ui::project::tests::a_load_that_fails_is_not_a_rebase` — the one its entry
describes — left the checker printing *all name a test that runs*, exit 0, with
the heading answered by the `ui::host::tests` namesake. Now an ambiguous leaf is
refused with both candidates named, and the two entries are qualified.
Re-verified both ways, 2026-09-21: the refusal fires on the unqualified
headings and names exactly those two; with the heading qualified, the same
rename is reported as an orphan rather than absorbed.

**`--rust-tests` was strictly more permissive and said the same words.** It
called `cargo_test_names` with one argument, so the `--ignored` subtraction
never ran, and the success line was byte-identical to the honest path's.
Injecting `### eyeball_the_rendered_hints_and_pages` orphaned on the default
path and passed on the flag's. Now the flag requires
`--rust-tests-ignored` (one alone exits 2) and the success line names its
source.

**A missing evidence directory exited 0.** The early return preceded the
citation scan, so deleting `context/test-evidence/` reported nothing while 170
citations pointed at a directory that was not there. Verified 2026-09-21 by
moving the directory aside: exit 1, *170 tests cite context/test-evidence and
have no entry there*.

### Note — the flag that selected the cap away (improvise-ijql)

`scripts/check-comment-runs.py --check-evidence` early-returned in `main`, so
the flagged run never answered the comment-run cap and printed nothing about
having skipped it. The reviewer's position when it was first raised was that CI
runs all three spellings (`ci.yml`, the three `check-comment-runs.py` steps in
the `test` job) so this was documentation only. The counter that carried is that
the flagged form is the **expensive** one — it needs a compiled workspace — which
makes it the one a human runs alone; three agents hit it independently in one
campaign, and in one case a local gate went green while CI went red on a
three-line doc comment.

Reproduced before the fix, 2026-09-21, in a temporary tree holding one `.rs`
file with a four-line comment run:

```
$ check-comment-runs.py src --check-evidence --rust-tests … --rust-tests-ignored …
test-evidence: 0 entries, all name a test that runs …
EXIT=0
$ check-comment-runs.py src
1 comment runs longer than 2 lines:
  src/bad.rs:1  4 lines
EXIT=1
```

Two invocations over one tree, opposite verdicts, and the one that exits 0 is
the one a human is more likely to run by itself.

The guard is `_dispatch_self_test`, which runs the script as a **subprocess** —
the only level at which this is visible, since every unit in the file was
correct and the defect was in `main`'s dispatch. Both arms were demonstrated:

| injected fault | what failed |
|---|---|
| restore the `--check-evidence` early return | `DISPATCH: --check-evidence: exited 0 over a four-line comment run` |
| make the cap always report a violation | `DISPATCH: bare: refused a clean tree` *and* the same for `--check-evidence` |

The second arm is what stops an always-failing cap from satisfying the first,
and it is why `SELF_TEST_DISPATCH` asserts a clean tree passes as well as a
dirty one failing. Why the fix is additive rather than a printed disclaimer, and
why `--self-test`/`--list-doctests` are excluded from the table rather than
exempted from the rule: `context/code/tooling.md` §2, "A flag adds a question".

### Note — the stamp gate's discrimination, measured

`scripts/assemble-stamp-test.sh` separates three outcomes rather than two, which
is what makes it evidence rather than a smoke test. Measured 2026-09-21 over its
nine cases: the real script passes 9; deleting the final `test -z "$missing"`
(the PR #69 injection, which the inline version survived at exit 0) passes 4 and
fails 5; a script that does nothing passes 2; one that always fails passes 0.

The PR body first cited "4 of 9" for the middle row, from a run taken when the
suite had eight cases. A figure is what the next reader checks, so it is
recorded here where it can be re-measured rather than only in a message nothing
can audit (design-principles §6).

### Note — the stamp gate matches literally

`scripts/assemble-stamp.sh` greps with `-F`. As a regex, the `.` in
`improvise_wasm_client.js?v=abc` matches any character, so a stamp corrupted in
exactly that position reads as present — a pass in the one direction a guard
must not fail in. Inherited from the inline version this was extracted from,
and found by reading the extraction rather than by anything failing.

Fault injected, 2026-09-21: dropping the `-F` makes
`improvise_wasm_clientXjs?v=abc` satisfy the pattern, and
`a pattern is matched literally, not as a regex` goes from pass to fail.

### Note — the doc site's derived checks

`scripts/build-doc-site.mjs` carries two gates that no test names, because the
generator *is* the test: it runs on every assembly and exits non-zero.

Faults injected, 2026-09-21, each observed:

| injected | reported |
|---|---|
| a new `context/code/newarea.md` | `context/code/newarea.md is neither published nor a named omission` |
| `context/plan.md` moved out of the tree | `context/plan.md is listed as omitted but does not exist` |
| a line appended below the generator in `web/assemble.sh` | `web/assemble.sh must invoke this generator LAST`, naming the offending command |
| a trailing `echo "see scripts/build-doc-site.mjs"` | the same, once the check asks for an INVOCATION rather than a mention — `.includes` had accepted it (review, PR #83) |

`scripts/check-ci-graph.py` is the third, and it is a script rather than a
stanza because its own refusals need checking: adding a `zz-new-check:` job to
`ci.yml` reports *pages-deploy does not need `zz-new-check`* and exits 1, where
before the rule was prose and nothing noticed. Its five self-test graphs include
the one a naive parse gets wrong — another job's `needs:` read as the deploy's.

The second arm is the one that would not get written first, and it is the point:
without it, deleting or renaming an omitted file leaves an entry asserting a
reason for a file nobody can find — the same failure as a dead ticket id, which
`AGENTS.md` already calls worse than no id.

The first arm was filed as latent and was live when it was written:
`context/code/kb-tool.md` had been unpublished, unargued and unnoticed since it
was added.


### Duplicate evidence headings — `SELF_TEST_DUPLICATES`

improvise-hsoe. `--check-evidence` verified that a heading names a test that
**runs**, and a duplicate heading names one just as well, so both copies pass.
PR #92 shipped two entries under
`a_layout_change_moves_the_layout_version_and_no_record`, and two more pairs
sat in `tui.md` — one of them asserting that `THE_MEMBERSHIP_CORRECT_SPANS` was
"a live tripwire, do not remove it" some four hundred lines above the entry
recording that improvise-gd83 had removed it. A reader meeting the older copy
first takes it as current, which is worse than an orphan: an orphan announces
itself as broken.

The fault is injected by the six cases in `SELF_TEST_DUPLICATES`, and three of
them are the ones that would not get written first. Two *different* tests
sharing a leaf must come back **ambiguous and not duplicate**, because the
wrong diagnosis files the wrong repair. A prose heading must be free to repeat,
because it claims no test. And the bare and module-qualified spellings of one
test must collide — grouping by the string rather than by the resolved test
would have missed exactly the case improvise-rm4s's qualification repair
creates, which is the first way this would have come back.

The live tree was itself the first injection: before the three merges the check
exited 1 naming all three pairs with both line numbers; after them it exits 0.
The clean arms (two headings naming two tests, and prose repeated) are what
stop an always-refuse from satisfying the rest.

### The PR review reader's own suite — `scripts/pr-reviews-test.sh`

improvise-xgra. Like `kb-test.sh` and `pr-watch-test.sh`, its cases are already
injected faults and none of their names is a Rust `fn` the evidence checker
could resolve, so the case list is the evidence. What is recorded here is the
five faults it was watched to fail against, and which case caught each — a
suite that only asked "did it print something" passes all five.

Fault 1, **the inline surface dropped entirely**, which is the live bug as it
shipped: `inline: []` in both fetch paths. Eight cases fail, including both
`--wait` arms. This is the one that matters, because the old script's behaviour
*was* this fault and it exited 0 on PR #122's review body while printing none
of the five rulings underneath it.

Fault 2, **an inline comment counted rather than compared**:
`((.inline|length) > 0)` in place of the `commit_id == head` test. One case
fails, `an inline comment at an EARLIER head does not satisfy --wait`, and that
is the pair the two inline fixtures exist to discriminate — the at-head case
passes under this fault, so on its own it proves nothing.

Fault 3, **the time sort dropped** (`| .` for `| sort_by(.at)`). Only
`the three surfaces interleave in time order` fails: it reads the three bodies
back as `SECOND THIRD FIRST`, the per-endpoint concatenation order. Every
content assertion still passes, because each body is still printed.

Fault 4, **no `original_line` fallback**. Only the outdated-comment case fails.
`line` is null exactly when the line has moved under the comment, so a renderer
reading `.line` alone prints `draw.rs:null` on precisely the comments whose
location is hardest to recover by hand.

Fault 5, **`--wait` widened to any issue-level comment**. Only
`an issue-level comment does not satisfy --wait` fails. That is the fault a
well-meaning repair would introduce — "wait for anyone to say anything" — and
the fixture that separates it is a PR thread comment with no `commit_id` at
all.

Two cases are there for the contract rather than the bug:
`a review body at the current head still satisfies --wait` and
`--wait 0 over an empty PR is success`, because other briefs depend on exit 0
and exit 1 meaning what they did before. `no PR number is exit 4` pins the
third code, which is distinguishable from 1 only if nothing collapses them.

`the usage warns about a trailing $? reader` asserts a sentence rather than a
behaviour, which is unusual and deliberate: the `; echo "exit=$?"` trap costs a
wrong diagnosis every time it is hit, the warning is the only guard against it,
and a header is exactly the thing a later edit trims without noticing.

Three more cases were added after ChatGPT Codex reviewed PR #133, and the pair
of bugs it found is the reason the suite has to run in CI rather than only on
the author's machine. Both produce **exit 2 with no output whatsoever**, which
is jq refusing an invalid `--argjson` — so a shell fault at the top of the
script arrives wearing jq's exit code, and 17 of 19 cases failed identically
with nothing to read.

The first, a multi-assignment `local` whose third clause reads its first, is
fine on macOS's bash 3.2 and fatal on bash 5 under `set -u`. **The suite passed
19/19 locally and 2/19 in CI on the same commit**, which is the case for wiring
it into the `kb` job in the same change rather than afterwards.

The second is `gh api --paginate` emitting one array per page. Fault 6 removes
`| pages_to_array` from the fixture path and exactly the three new cases fail —
`a comment on the first page survives pagination`, `…the second page…`, and
`a paginated thread still satisfies --wait` (exit 2, the live signature). The
fixture carries two concatenated arrays and both paths flatten through the same
helper; a fixture holding one array could not have caught this, because it
would exercise a shape the live fetch never produces. No PR in this repo has
yet had enough inline comments to reach a second page, so the test is the only
thing that will ever see it before a user does.

### The checker runner's own suite — `scripts/checkers-test.sh`

The bug behind improvise-f9qf was **invisible to a local run**, so the evidence
that matters here is not "the suite is green" but "the suite goes red when the
fault is put back, in the place the fault was not seen."

**The fault, injected against the real script.** Restoring `pr-reviews`'
multi-assignment `local` — the exact text `051f541` replaced — and running the
suite three ways on one machine in one sitting:

| gate | result |
|---|---|
| `sh scripts/pr-reviews-test.sh` (the old local gate) | 22 passed, 0 failed |
| `direnv exec . ./scripts/pr-reviews-test.sh` | 22 passed, 0 failed |
| `./scripts/checkers.sh pr-reviews` | **2 passed, 20 failed**, exit 1 |

The middle row is the one that earns the design. The dev shell supplies the
*correct* bash and would still have shipped this, because `pkgs.mkShell` exports
`name` and the broken clause read `$name`. Only scrubbing the environment as
well as pinning the shell makes the fault visible, and the 2/20 shape matches
the 2/19 CI actually reported.

**`checkers-test.sh` itself is twelve cases, and each injects rather than
asserts.** The two that carry the argument:

- *a suite reading an inherited dev-shell variable fails under the runner* — a
  fixture suite whose body is `echo "$name"` under `set -euo pipefail`, run with
  `name=nix-shell-env` exported, exactly as direnv leaves it. It is paired with
  *and passes when run directly with that variable exported*, which asserts the
  **same fixture passes the old way**. Without that pair the first case is a
  test of `set -u` and proves nothing about the runner.
- *a bash-3.2-only construct fails under the runner's pinned bash* — a fixture
  suite containing `local a="$1" b="$a/x"`, which is the bug's shape reduced to
  two lines. Green on bash 3.2, fatal on bash 5.

The rest separate the runner's outcomes from each other: a `#!/bin/sh` script in
the directory is refused **and the same sandbox passes once it is removed** (so
the refusal is not standing in for some other complaint); a failing suite fails
and a passing one passes; an old bash with no nix is exit 3 rather than a quiet
run; an unknown suite name is exit 4 rather than a silent no-op; and `--list`
names exactly the suites present, including the case where an `X-test.sh` has no
`X` beside it — `scripts/node-test.sh` is a test *runner*, and the first version
of the glob ran it as a suite, where it printed its usage inside a green gate.

**What is asserted about the runner's own environment, and what is not.** The
allowlist is checked only in the sense that seven real suites pass through it;
there is no case asserting a *particular* name is absent beyond `name`. A suite
that grows a dependency on some other inherited variable will fail loudly on
first run rather than be caught here. Stated rather than guarded.

### The two-workspace fmt gate — `scripts/fmt-workspaces-test.sh`

The bug (improvise-b4ks) was a **gate decoupled from what it checks while
wearing the exit code of a real failure**. From the repo root,
`cargo fmt -p opfs-contract -- --check` exits **1** with `package
'opfs-contract' is not a member of the workspace` — `--check` never runs. A
caller reading `$?` cannot tell "this crate is misformatted" from "nothing was
checked", so the gate could neither report success when the crate was dirty nor
failure when it was clean. Reproduced at `7cef9f8`, exit 1, before any fix.

`scripts/fmt-workspaces.sh` replaces it: it discovers every `[workspace]` root
and checks each with `-p <member>` scoped by that root's own `--manifest-path`.
Exit **2** is a usage error or a cargo that could not run; **1** is a formatting
verdict; they are never the same number.

**Why `-p` and not the two obvious spellings** — both were measured, and both
fail in a direction that looks like success:

- `--manifest-path <root> --all` **crosses the workspace boundary**. `--all`
  formats "all packages, and also their local path-based dependencies", and
  `harness/opfs-contract` depends on `improvise-io` by path, so the inner root
  reports the *outer* tree's files. Measured: an injected fault in `src/draw.rs`
  was reported by the harness root's own invocation.
- `--manifest-path <root>` bare is **blind to the members**. The repo root is a
  package (`improvise`) as well as a workspace, so the bare form checks only
  that package. Measured: an injected fault in `crates/improvise-core/src/format.rs`
  exited **0**.

**Four rivals were injected into the script and each failed the suite**:
`--all` for `-p` (the boundary leak, caught by counting — the outer file must be
named exactly once); the bare manifest-path (caught by the outer-fault case);
`return 2` collapsed to `return 1` on the usage arms (two cases); and "no
workspace root found" returning 0 instead of 2.

**The fixture had to be made faithful twice, and each vacuity was found only by
injecting the rival — not by reading.** The suite passed under a rival at both
stages, which is the only reason the gaps were visible:

- The first fixture's nested crate had **no path dependency** into the outer
  tree, so `--all` had nothing to leak through and the boundary rival passed.
- The first fixture's outer root was a **virtual manifest** with no `[package]`,
  where the bare form happens to reach every member, so that rival passed too.

Both are properties this repo really has, and a fixture that omits them tests a
repository that does not exist. That is the generalisable point: **a
check_the_checker's fixture must reproduce the structural features the hazard
depends on, and the only way to know it does is to inject the rival and watch
it fail.** A green suite over a simplified fixture is the same non-evidence as a
green suite over a vacuous assertion.

**What is not covered.** The script asks `cargo metadata` for members, so a root
whose metadata succeeds but whose member list is wrong is not detectable here.
*Narrowed by improvise-90ea, below:* a list that **omits** a package in the tree
is now refused; one that names a package it should not is still not detected.
And `harness/opfs-contract`'s own `cargo fmt -p opfs-contract` step in the
`harness-lint` job is left as it is: it resolves correctly because that step
sets `working-directory`, and the ticket's own note confirms CI as written was
never the broken caller.

#### The exit contract, narrowed after review — `scripts/fmt-workspaces-test.sh`

ChatGPT Codex on PR #159 found that the contract above promised more than the
script maintained: both formatter invocations end `|| rc=1`, so **any** failure
after the `cargo fmt --version` probe was reported as "misformatted" even
though no verdict had been produced. The finding was correct and was measured
rather than assumed:

- an **unparseable source** (`pub fn a( {{{`) exits **1** — identical to a diff;
- an **unknown `rustfmt.toml` option** is only a warning, and exits **0**.

Of the two remedies offered, the contract was **narrowed** rather than the
failure re-classified, because parsing rustfmt's prose to tell a diff from a
parse error is precisely the check-adjacent-to-its-subject this repo keeps
getting wrong. The line the split actually needs is **"did the gate run"**:

- exit 1 — rustfmt did not certify a root clean. A diff or a refusal to parse;
  either way **a file is named** and the tree is what is wrong.
- exit 2 — the gate did not run. Usage, no cargo, no rustfmt, `cargo metadata`
  failed, no `[workspace]` root. **No source file is named.** Every such cause
  is checked *above* the loop, which is what makes the narrowing true rather
  than hopeful. (Since improvise-90ea, a package no root reaches is also exit 2,
  and it names that package's `Cargo.toml`, which is the reason no verdict
  exists. The line is "no verdict", not "no output"; improvise-ximf.)

Both directions are now pinned, and two further rivals were injected:

- deleting the `cargo fmt --version` probe;
- turning `|| rc=1` into `|| return 2`, which collapses a verdict into the
  usage code — the mirror of the original bug, and caught by three cases.

**The first of those exposed a vacuous test, again only by injection.** The
original stub was a `cargo` that failed for every argument — but such a cargo
also breaks `cargo metadata`, so the run exited 2 on the *metadata* arm and the
probe could be deleted with the suite still green. A missing rustfmt component
is `cargo metadata` **succeeding** while only `cargo fmt` fails, so the stub now
delegates every other subcommand to the real cargo and fails `fmt` alone. Both
stubs are kept, because they exercise different arms.

That is the third fixture correction on this one checker, all three found the
same way: the suite passed *under a rival*. The generalisable rule is that a
stub must fail in the **shape** of the real fault, not merely fail — a stub that
breaks more than the real thing breaks satisfies the test through the wrong arm,
and reads exactly like coverage.

#### The root package, and why the fourth fix is structural — `scripts/fmt-workspaces-test.sh`

A post-merge review of PR #159 found a **fourth** fixture vacuity
(improvise-90ea). The fixture's outer root is a package (`root-pkg`) as well as
a workspace, which is right, but no case ever dirtied **the root package's own
`src/lib.rs`**. Only the member `a/` and `nested/` were dirtied. So a script
that formats every member *except* the root package passed 17/17. In this repo
the root package is `improvise`, the whole TUI. Reproduced at `40b8913` with
the review's rival in `members_of` (drop the package whose manifest sits at
`workspace_root`): the suite passed 17/17 and `--list` left `improvise` out.

Adding one more hand-picked case would have fixed the fourth and not the fifth.
Four vacuities in one checker all have the same cause: **the cases were chosen
by hand, one per hazard somebody had thought of.** So the fix is structural,
and it has two halves because it has to cover two sets of packages:

- **Every package the fixture has.** The two hand-picked fault cases are
  replaced by a loop over every `[package]` manifest in the fixture. Each one
  gets its own misformatted `src/lib.rs`, must exit 1, and must be named
  **exactly once**, so the `--all` boundary check now covers every package
  rather than one. The enumeration is a `grep` for `[package]`, deliberately
  *not* the script's `[workspace]` + `cargo metadata` discovery, since that
  discovery is what is under test. A count assertion (three packages) stops
  the loop going vacuous if the fixture loses a package. A package added to
  the fixture gets its case automatically.
- **Every package the tree has.** The loop proves only that the script handles
  the fixture's *shapes*. A real tree can hold a package no root lists (an
  `exclude`d crate, say), and nothing in the fixture would say so. So the gate
  itself now checks an invariant **before it formats anything**: every
  `[package]` manifest the find reaches must be a member of some root, or it
  exits **2** and names the package. That is exit 2 and not 1 because no
  verdict was produced for that package, which is what exit 2 means here.
  Paths are compared through `realpath`, because on macOS `mktemp` lives under
  `/var` while cargo reports `/private/var`.

**Rivals injected, and where each failed** (the suite went from 17 to 21 cases):

- **The review's skip-the-root rival.** It now fails the fixture at `a fault in
  src/lib.rs is reported` (exit 0). With the invariant in place it fails
  **six** cases (exit 2, "no workspace root reaches"). It also fails the
  **real repo** with no fixture involved: `fmt-workspaces.sh` exits 2 naming
  `Cargo.toml`. That last one is the property worth having. The gate refuses
  its own blindness in CI, not only in its test suite.
- **Disabling the invariant** (`if false`). It fails `a package no workspace
  root reaches is exit 2, not a pass` with **exit 0**, even though that
  package's `src/lib.rs` is misformatted. That exit 0 *is* the hazard: an
  unreached package is silently clean.
- **The two earlier rivals, re-run against the loop.** `--all` for `-p` fails
  both `a/src/lib.rs` and `src/lib.rs` "named exactly once". The second of
  those was not predicted: the nested root's `--all` also reaches the root
  package. The bare manifest-path form fails `a fault in a/src/lib.rs is
  reported`.

The real-repo control from the ticket still holds: a misformatted fn appended
to `src/ui/text.rs` is reported (`Diff in …/src/ui/text.rs:72`).

**What is still not covered.** A root whose member list *includes* something it
should not is not detected, and a package in a directory the find excludes
(`target/`, `.claude/`, `node_modules/`) is outside the invariant by
construction.

### The kb tool's completeness guard and atomic write — `scripts/kb-test.sh` cases 18–22

improvise-usus: `scripts/kb add` wrote onto a working-tree `kb.pl` that had lost
276 of its 801 clauses, and `kb_consistent` passed. The argument for the fix
(compare with HEAD by key, record `kb rm` removals in a ledger, publish by
rename) is in `context/code/kb-tool.md`. This entry covers what each case was
watched to catch.

**Reproduced at `40b8913` before any fix.** A copy of HEAD's `kb.pl` with 34 of
its 35 `location/3` facts removed still passes `kb_consistent`, and
`kb add` onto it printed `775 before, 1 in delta, 776 after` and exited 0.
Against the unfixed tool, the new cases gave 33 passed, 5 failed: 18, 19, 20b,
21 and 22 failed. Case 20 passed there because an rm followed by an add has no
guard to trip.

**The fixture has to fail in the incident's shape.** The ticket's "truncate to
half" fixture also breaks `kb_consistent`, because the rules live in the second
half. So the unfixed tool refused it too, by the wrong guard. Removing a whole
predicate is an existence error, and removing cited clauses fails the citation
check. Case 18 keeps one `location/3` fact and asserts that the fixture passes
`kb_consistent` before using it. Case 19 keeps the ticket's half-truncation, and
`said()` is what makes it evidence: the completeness guard runs first and must
be the one that answers.

**Rivals injected into `scripts/kb`, one at a time. Each failed only its own
case:**

- baseline taken from the file's own pre-write state rather than HEAD: 18, 19
  and 20b failed (35/38 passed). This is the guard that would have passed the
  real incident.
- ledger never consulted: 20 and 20b failed. A legitimate rm then blocks every
  later write.
- guard stands down whenever the ledger is non-empty: 20b failed.
- candidate built by plain `mktemp` in `$TMPDIR`: 22 failed on the directory
  check.
- candidate copied over `kb.pl` before the publish point: 22 failed as
  modified.
- unreadable baseline exiting 1 rather than 3: 21 failed.

**A rival can fail wider than its fault.** The first form of the "stands down"
rival skipped `merge complete` altogether. That also skipped writing
`$cand.ledger`, so the later `mv` crashed under `set -e` and five unrelated
cases failed. It proved nothing until it was rewritten to carry the ledger
forward.

**Checked by hand, not in the suite:** the ledger prunes an entry once the key
is re-added and once HEAD stops having it (a committed removal).

### The kb completeness guard's follow-ups — `scripts/kb-test.sh` cases 19b, 20b, 23–25

improvise-ih3k, improvise-yea8 and improvise-za97, filed by the review of PR
#166. The argument is in `context/code/kb-tool.md`. This entry covers what each
case was watched to catch.

**Reproduced at `f909cad` before any fix.** The new cases 23, 23c and 24 gave
40 passed, 3 failed. 23 failed because an add onto a kb that had lost one of
two `work_done(<fixture id>,bug_fix,…)` clauses exited 0. 23c failed
because `rm` of that paired key was refused as "a clause was dropped": its
expected total assumed one clause per key. The tickets did not predict that
one, and it meant a pair could not be resolved through the tool at all. 24
failed because the stale ledger entry excused the loss.

**The ticket's own reproduction defeats a content stamp.** The first fix
stamped the ledger with the kb's sha256, and case 24 still failed. `kb.pl` is
canonical, so `grep -v` of one clause from HEAD is byte-identical to what `kb
rm` of that clause writes. The stamp therefore also carries the inode and the
nanosecond mtime.

**Case 20b had to change, and why that is not a weakening.** It made its
second loss with `sed -i` on the kb. Under the fix, any edit outside the tool
voids the ledger, so 20b then refused two clauses instead of one and failed on
its "1 of" assertion. It was testing the new voiding rather than what its
comment names. The second loss now comes from giving the *baseline* an extra
clause (`KB_BASELINE`). The kb file stays exactly as the tool wrote it, the
ledger stays valid, and the case again isolates "excuses only what it names".
It also now asserts the unexcused clause is named.

**Rivals injected, one at a time. Each failed only the cases listed:**

- keys compared as a set (`min(1, N)` per key) in `complete`: 23 only.
- `rm`'s expected total back to `before - |delta|`: 23c's rm step only.
- ledger pruned by key presence rather than count: 23c's final write only.
- ledger honoured whenever it exists: 24 and 25.
- ledger stamped by content alone: 24 only. 25 cannot see it, because its
  intervening `add` makes the restored-then-lost file differ in content
  anyway. 24 is the case that carries identity.
- ledger re-stamped only by `rm`, keeping the old stamp on other writes: 24b
  (as named), 20b and 23c. All three write again after an add.
- ledger stamped with the kb as read rather than the candidate: 20, 20b, 23c,
  24b and 25. That is wider than any one case, as expected, since no later
  write can ever match.
- the `gh/main` `scripts/kb` (old refusal text, no stamp): 19b, 24 and 25.

**Case 25 is the first to run the production paths.** Every other case sets
`KB_FILE`, so `ledger_path`'s `git rev-parse --git-path` branch and
`baseline_to`'s `git show HEAD` were unexercised. It copies the tool into a
throwaway `git init` repo and checks four things. `rm` records to the git-path
ledger. `git status` shows only `kb.pl` modified, so the ledger is invisible
to git. A later `add` is excused. After `git checkout --` and a hand loss, the
`add` is refused.

**Checked by hand, not in the suite:** a `SIGKILL` at the publish point (the
`KB_TEST_KILL` seam) on a copy inside `prolog-sessions/` leaves the candidate,
its `.base`, `.ledger`, `.removed` and `.stamped` siblings and `.lock`. `git
status --untracked-files=all` and `git add -A --dry-run` both show none of
them, and `git check-ignore -v` names `.gitignore:40` (`prolog-sessions/*`).

### The pr-watch mergeability cases — `scripts/pr-watch-test.sh`

improvise-edpl. The cases are injected faults in themselves. Like the other
`scripts/` suites, none of their names is a Rust `fn`, so the case list is the
evidence. The reasoning is in `context/code/tooling.md`, "pr-watch and
mergeability".

**The bug, reproduced before the fix.** Against the script as it stood at
`ae96b8c`, 19 of the 34 new cases failed and all 19 exited **0**.
`UNKNOWN that resolves to CONFLICTING fails` is PR #172's shape exactly: every
check green, `UNKNOWN/UNKNOWN` on the first poll, `CONFLICTING/DIRTY` on the
second. The old script broke out of the loop on the first poll and passed. The
fixtures are hand-built. They are the subset of the `gh pr view --json` shape
that the script reads: `headRefOid`, the two mergeability fields, and the
rollup's `status` and `conclusion`. They are not a captured payload. A stub that
errors instead would have exited 4, a wider shape than the fault.

**Rival 1: treat `UNKNOWN` as an answer.** The wait condition was replaced with
`if true`. 13 cases fail: every `UNKNOWN` row of the table,
`MERGEABLE/UNKNOWN`, the timeout case, and
`UNKNOWN that resolves to MERGEABLE/CLEAN passes`. That last one exits 1 under
this rival, so the fix cannot pass by refusing everything uncertain. It has to
wait. `UNKNOWN that resolves to CONFLICTING fails` still passes under this
rival, because the allowlist refuses `UNKNOWN` as well. Rival 2 does not
separate it either: its first poll is `UNKNOWN`, which rival 2 still waits on.
What separates it is the original fault, which combined both rivals. An earlier
version of this entry said rival 2 separated it. That was wrong.

**Rival 2: the old verdict.** The allowlist was replaced with `exit 0` after the
`MERGEABLE` test. Five cases fail: `MERGEABLE/BEHIND`, `DIRTY`, `DRAFT`,
`SOMETHING_NEW` and `null`. These are the states that look mergeable by the flag
and are not. At `128a2a0` it was six, with `BLOCKED` among them. Since
improvise-zlj2, `BLOCKED` is waited on, so this rival never reaches the verdict
for it. The entry below covers that.

`a failed check fails without waiting on mergeability` runs at `--timeout 0`.
A script that waited on `UNKNOWN` before reading the rollup would report that
known red as exit 3.

### The pr-watch needs-gap and pinned-head cases — `scripts/pr-watch-test.sh`

improvise-zlj2 and improvise-h3ni. These were found by review of PR #186 at
`128a2a0`, which was ruled do-not-merge.

**zlj2: a healthy PR exited 1.** The required `Browser harness` check run
exists only after the `build` job completes (`needs: build`). In that gap
GitHub reports `MERGEABLE/BLOCKED` while every registered check is green, and
the #186 rule refused `BLOCKED`. The rollup in the new cases is PR #189's,
captured verbatim with `gh pr view --json`. It has thirteen entries with every
field GitHub returns. Each gap state drops or rewinds entries of that rollup
with `jq`. Against `128a2a0` three cases failed, all with exit 1:

- `the needs-gap before the required check registers is not a failure`: the
  healthy sequence should exit 0.
- `a required check that never registers is a timeout`: should exit 3.
- `table: MERGEABLE/BLOCKED`: should exit 3, not 1.

**Rival for zlj2:** remove `MERGEABLE/BLOCKED` from the wait. The same three
cases fail with exit 1. That rival is `128a2a0`'s code.

**h3ni: nothing pinned the head across the UNKNOWN wait.** The case is
`a head that moves during the UNKNOWN wait is not a pass`. Its polls are
UNKNOWN@a, then UNKNOWN@b, then CLEAN@b, and it must exit 2. It passed on the
first run, so it is a guard for behaviour the script already has, not a
reproduction.

**Rival for h3ni:** re-pin the head whenever `mergeable` is UNKNOWN. That case
alone fails, exiting 0. All 44 others pass. That is the review's point: the
older `a head that moves mid-wait is not a pass` starts from MERGEABLE, so it
cannot see this rival.

## Every JSON parser reads numbers exactly (improvise-dz8c)

### `every_crate_that_can_parse_json_enables_float_roundtrip`

It replaces a check over three crates listed by hand. It reads
`cargo metadata --no-deps` and fails naming every workspace package, and
every target-specific entry, whose **normal** serde_json dependency lacks
`float_roundtrip`. The rule and why it was chosen over the alternatives are in
`context/code/protocol.md`. It is red at `1158c23`, naming `improvise`,
`improvise-command`, `improvise-io` and `improvise-protocol`.

**Faults injected, each run:**

- **The feature removed from `improvise-io` alone.** The check fails naming
  `improvise-io (all targets)`, and so does
  `a_legacy_json_workbook_loads_its_numbers_bit_for_bit`.
  `a_json_import_stores_its_numbers_bit_for_bit` stays green, because
  `improvise-command` enables the feature and unification reaches io in its
  build.
- **The feature removed from `improvise-command` alone.** The check fails naming
  `improvise-command (all targets)`. The command crate's own import test
  **stays green**: `cargo tree -p improvise-command` shows io's declaration
  switching the feature on. So the manifest check is the only guard that
  sees this case. It is also the case the retired build-graph check would
  have passed.
- **A throwaway workspace member**, `zz-throwaway`, with normal dependencies on
  `improvise-protocol` and `serde_json` and no feature (the lockfile updated,
  as a real new crate would be). The check fails naming
  `zz-throwaway (all targets)`. With the unconditional dependency carrying the
  feature and a `cfg(target_arch = "wasm32")` one lacking it, the check fails
  naming `zz-throwaway (cfg(target_arch = "wasm32"))`. That is stricter than
  the build needs, since a wasm build unifies the two entries, and cheap to
  satisfy. The crate was then removed.

The `checked.len() >= 2` assertion is the positive signal. If a cargo change
renamed the metadata fields, the check would find no serde_json dependency and
pass vacuously without it.