diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19349628..4646ac2d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,8 +280,10 @@ jobs: # was blind to inline comments for as long as it existed (improvise-xgra). # fmt-workspaces is here because its own failure mode is the bug it # exists for: a usage error wearing a formatting failure's exit code. + # bd-audit-ids is the dead-ID audit, whose recipe once read a bd that + # never ran as a clean audit (improvise-52eq). - name: The remaining checkers refuse what they claim to - run: ./scripts/checkers.sh pr-watch release-preflight pr-reviews fmt-workspaces + run: ./scripts/checkers.sh pr-watch release-preflight pr-reviews fmt-workspaces bd-audit-ids # --------------------------------------------------------------------------- # Lints the one crate the `test` job cannot reach. diff --git a/AGENTS.md b/AGENTS.md index 6e66e82c..c2a1fa36 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -293,18 +293,19 @@ session before an agent tried one and found nothing. Read the ID back after filing, and before pushing docs run the audit: ```bash -git ls-files -z --cached --others --exclude-standard -- \ - context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \ - .agents .claude \ - | xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' \ - | sort -u \ - | grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$' \ - > /tmp/bd-audit-ids.txt -bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \ - | grep -E '^(Issue [^ ]+ not found|[Nn]o such issue)' || echo "all resolve" +./scripts/bd-audit-ids ``` -`git ls-files` picks the files, and grep only reads them. The file list is the +It exits 0, printing `all N cited ids resolve`, only when every cited id is in +what `bd list --all` returns. A dead id is exit 1, and each one is named. A bd +that is missing, fails, or lists no issues is exit 3. The comparison is +positive because the recipe the script replaced was not: it searched `bd show` +for "not found" lines and printed "all resolve" when there were none, so a bd +that never ran read as a clean audit (improvise-52eq). Its suite, +`scripts/bd-audit-ids-test.sh`, runs in `checkers.sh` and checks both halves: +a dead id, and a bd that does not answer. + +In the script, `git ls-files` picks the files, and grep only reads them. The file list is the tracked files plus the untracked ones, minus anything git ignores. That leaves out two things a `grep -r` over the same roots reads: @@ -328,9 +329,6 @@ follows those symlinks and reads the skills a second time (3107 matches). Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b` and finds a fraction of the ids. -The last line matches at the start of a line because `bd show` prints each -ticket's body indented, and a body can itself contain the words "not found". - Three things the earlier version of this recipe got wrong, each of which let a real defect through: @@ -344,7 +342,7 @@ real defect through: to match. An ID-shaped non-ID is worse than a dead ID, because the diagnosis is identical and the repair is not. - **The exclusion list named only `core` and `wasm`**, so every other crate - name would have been reported missing. The exclusions above also cover the + name would have been reported missing. The script's exclusions also cover the non-ticket tokens this repo really contains: a package name, a temp file name, the prose word "improvise-side", and the example crate name in the `kb` skill's `crate(...)` fact. **They are anchored `^…$` for the @@ -357,7 +355,7 @@ real defect through: - **A release artifact's name is ID-shaped, and `improvise-installer.sh` escaped only by being ten characters long.** `improvise-web.tar.gz` (improvise-oujb) is three, so it matches `{3,8}` and resolves to nothing; - `web` is therefore excluded above. The general point is that the length + `web` is therefore excluded in the script. The general point is that the length bound, not any rule, is what has been keeping `dist`'s artifact names out of this audit — so **a new artifact whose name is a short lowercase word must be added to the exclusion list in the same commit**, or it arrives as a dead id @@ -368,11 +366,12 @@ lists example ids makes itself a source of audit noise. The ids it does cite, improvise-8haj and improvise-oujb, are the tickets behind two of the rules above. They resolve, and the audit checks them like any other. -One environment note: in a worktree-isolated agent the shell guard refuses a -loop that passes a runtime-computed argument to `bd`, so the recipe above -materialises the id list into a file and hands `bd show` the whole list in one -call rather than looping. If `bd show` in your build does not take several ids, -run the ids through `xargs -n1 -I{} bd show {}` from a plain shell instead. +One environment note: run the audit by its name. Which `bd` command forms a +worktree-isolated agent's shell guard allows has varied between sessions. In +one session it refused the old recipe's `bd show $(cat …)`, and also +`xargs bd show < file` and `sh file`. In another, both `xargs` forms ran. +`./scripts/bd-audit-ids` passes `bd` no argument computed at runtime, and it +ran in the session that refused the other three. A dead ID is worse than no ID: it reads as a promise that the follow-up exists. diff --git a/context/test-evidence/tooling.md b/context/test-evidence/tooling.md index c6845bd5..8ac79c4f 100644 --- a/context/test-evidence/tooling.md +++ b/context/test-evidence/tooling.md @@ -811,3 +811,39 @@ byte for byte (`cmp` against the committed file). With the committed lock, to corrupt a tracked file and call cargo, which needs the crates.io index, from `checkers.sh`, whose environment is scrubbed. The by-hand run above is the evidence. + +### The dead-ID audit compares positively — `scripts/bd-audit-ids-test.sh` (improvise-52eq) + +**What broke.** The audit recipe in `AGENTS.md` ended +`bd show $(cat ids) 2>&1 | grep -E '^(Issue … not found|…)' || echo "all resolve"`. +It looked for the *absence* of a negative line. So a bd that was missing, +failed, or never ran printed no such line, and the audit reported "all +resolve". In this session one attempt at the audit printed "all resolve" +because its output file did not exist. + +**The fix.** `scripts/bd-audit-ids` keeps the file scan and the exclusions as +they were. It then takes the known ids from `bd list --all --limit 0 --json` +and `comm -23`s the cited set against them. It exits 3 if bd is missing, fails, +prints no JSON list, or lists no issues. On the tree at `12c81dd` it reports +460 cited ids, the same count the old pipeline produced there. + +**The suite** runs a throwaway git repo and a fake bd that answers only `list`. +It has eight cases: a pass, a dead id (exit 1, named), four shapes of a bd that +does not answer (no output, `[]`, a failing exit, not on PATH), the real tree +against a silent bd, and `--help`. The passing fixture also cites +`improvise-core` and keeps a dead id in a git-ignored `harness/serve/` file. +So the pass needs both the exclusions and git's file list. + +**Rivals, each run:** +- **The old recipe**, pasted in as the script's tail: 1 passed, 7 failed. + Every "bd did not answer" case exited 0 with "all resolve", which is the + bug. The dead-id case failed only because the fake bd refuses `show`. That is + one more bd that did not answer, so it is not evidence the old recipe misses a + dead id against a working bd, which it catches. +- **The dead-id comparison disabled** (`: >` for `comm -23`): only "a dead id + is exit 1, and named" failed, with exit 0. +- **An empty exclusion list**: only the passing case failed, reporting + `improvise-core` as dead. + +After each rival the original was restored (`cmp` against a saved copy) and the +suite re-run at 8 passed. diff --git a/scripts/bd-audit-ids b/scripts/bd-audit-ids new file mode 100755 index 00000000..5f871cb4 --- /dev/null +++ b/scripts/bd-audit-ids @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# The dead-ID audit: every improvise- the tree cites must be a ticket bd +# knows. See AGENTS.md, "Cite a ticket ID you have read back" (improvise-52eq). +set -euo pipefail + +usage() { + cat >&2 <<'EOF' +usage: bd-audit-ids + +Run from anywhere inside the repository. Takes no arguments. + +exit 0 every cited id is a ticket bd lists +exit 1 at least one cited id is not; each is printed +exit 3 bd could not answer: missing, failing, or listing nothing +exit 4 usage error, or the file scan itself failed + +The comparison is POSITIVE: a cited id passes only by appearing in what +`bd list --all` returns. The recipe this replaced looked for bd's "not found" +lines and reported "all resolve" when there were none, so a bd that never ran +read as a clean audit. + +THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same +pipeline or `;` list. +EOF + exit 4 +} +[ $# -eq 0 ] || usage + +BD="${BD_AUDIT_BD:-bd}" +ROOTS=(context README.md AGENTS.md CLAUDE.md crates src tests examples web + harness docs .agents .claude) +# Crate, package, artifact and file names that are shaped like ids. Anchored, +# because an unanchored `gp` would silently exclude any real ticket id starting with it. +EXCLUDE='^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$' + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +cd "$(git rev-parse --show-toplevel)" +git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" > "$work/files" + +# xargs exits 123 when one grep batch matched nothing, which is not an error. +set +e +xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw" +rc=$? +set -e +[ "$rc" -eq 0 ] || [ "$rc" -eq 123 ] || { echo "bd-audit-ids: grep failed ($rc)" >&2; exit 4; } +sort -u "$work/raw" | { grep -vE "$EXCLUDE" || true; } > "$work/cited" + +if ! command -v "$BD" >/dev/null 2>&1; then + echo "bd-audit-ids: '$BD' is not on PATH, so nothing was checked." >&2 + exit 3 +fi +if ! "$BD" list --all --limit 0 --json > "$work/known.json" 2> "$work/bd.err"; then + echo "bd-audit-ids: '$BD list' failed, so nothing was checked:" >&2 + cat "$work/bd.err" >&2 + exit 3 +fi +if ! jq -r '.[].id' "$work/known.json" 2>/dev/null | sort -u > "$work/known"; then + echo "bd-audit-ids: '$BD list' did not print a JSON list of issues." >&2 + exit 3 +fi +if [ ! -s "$work/known" ]; then + echo "bd-audit-ids: '$BD list' returned no issues; refusing to call that a pass." >&2 + exit 3 +fi + +comm -23 "$work/cited" "$work/known" > "$work/dead" +if [ -s "$work/dead" ]; then + echo "bd-audit-ids: cited but not a ticket bd knows:" + sed 's/^/ /' "$work/dead" + exit 1 +fi +printf 'bd-audit-ids: all %d cited ids resolve (bd knows %d)\n' \ + "$(wc -l < "$work/cited" | tr -d ' ')" "$(wc -l < "$work/known" | tr -d ' ')" diff --git a/scripts/bd-audit-ids-test.sh b/scripts/bd-audit-ids-test.sh new file mode 100755 index 00000000..933980db --- /dev/null +++ b/scripts/bd-audit-ids-test.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# check_the_checker for scripts/bd-audit-ids. A throwaway git repo and a fake +# bd; no network, no real tracker. See context/test-evidence/tooling.md. +set -uo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +AUDIT="$here/bd-audit-ids" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +pass=0; fail=0 +ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); } +bad() { printf ' FAIL %s (%s)\n' "$1" "$2"; fail=$((fail+1)); } + +# The fake bd answers `list` from $FAKE_BD_OUT and exits $FAKE_BD_RC; any +# other subcommand is an error, so the audit cannot pass by calling `show`. +mkdir -p "$work/bin" +cat > "$work/bin/bd" <<'EOF' +#!/usr/bin/env bash +[ "${1:-}" = list ] || { echo "fake bd: unexpected '$*'" >&2; exit 2; } +[ -f "${FAKE_BD_OUT:-}" ] && cat "$FAKE_BD_OUT" +exit "${FAKE_BD_RC:-0}" +EOF +chmod +x "$work/bin/bd" + +repo="$work/repo" +mkdir -p "$repo/context" "$repo/harness/serve" +git -C "$repo" init -q +printf 'serve/\n' > "$repo/harness/.gitignore" +# A live id, and a crate name the exclusion list must drop: without the +# exclusion, improvise-core would be reported dead and the first case fails. +printf 'See improvise-ab12 in improvise-core.\n' > "$repo/context/notes.md" +# Ignored build output with a dead id. The audit reads git's file list, so +# this must never be reported (improvise-8haj). +printf 'improvise-gone9\n' > "$repo/harness/serve/copy.md" + +printf '[{"id":"improvise-ab12"},{"id":"improvise-cd34"}]\n' > "$work/known.json" +printf '' > "$work/empty" +printf '[]\n' > "$work/none.json" + +# $1 name, $2 expected exit, $3 grep -E pattern the output must contain; +# the caller sets FAKE_BD_* and BD_AUDIT_BD in the environment. +run_case() { + local name="$1" want="$2" pat="$3" + (cd "$repo" && "$AUDIT") > "$work/out" 2>&1 + local got=$? + if [ "$got" != "$want" ]; then bad "$name" "exit $got, wanted $want: $(head -c 200 "$work/out")" + elif ! grep -Eq "$pat" "$work/out"; then bad "$name" "no /$pat/ in: $(head -c 200 "$work/out")" + else ok "$name"; fi +} + +export BD_AUDIT_BD="$work/bin/bd" + +FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0 \ + run_case "every cited id known is a pass" 0 'all 1 cited ids resolve' + +printf 'Also improvise-zzzq.\n' > "$repo/context/dead.md" +FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0 \ + run_case "a dead id is exit 1, and named" 1 '^ improvise-zzzq$' +rm -f "$repo/context/dead.md" + +# The three shapes of "bd never answered". The recipe this replaced reported +# "all resolve" for every one of them (improvise-52eq). +FAKE_BD_OUT="$work/empty" FAKE_BD_RC=0 \ + run_case "a bd that prints nothing is not a pass" 3 'did not print a JSON list|returned no issues' +FAKE_BD_OUT="$work/none.json" FAKE_BD_RC=0 \ + run_case "a bd that lists no issues is not a pass" 3 'returned no issues' +FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=1 \ + run_case "a bd that fails is not a pass" 3 'list. failed' +BD_AUDIT_BD="$work/bin/no-such-bd" \ + run_case "a missing bd is not a pass" 3 'is not on PATH' + +# The live repo, with the fake bd listing nothing: if this passed, a real +# audit whose bd fell silent would pass too. +(cd "$here/.." && FAKE_BD_OUT="$work/none.json" "$AUDIT") > "$work/out" 2>&1 +got=$? +[ "$got" = 3 ] && ok "the real tree does not pass against a silent bd" \ + || bad "the real tree does not pass against a silent bd" "exit $got" + +"$AUDIT" --help > "$work/out" 2>&1 +got=$? +[ "$got" = 4 ] && grep -q 'POSITIVE' "$work/out" \ + && ok "--help is exit 4 and states the positive comparison" \ + || bad "--help is exit 4 and states the positive comparison" "exit $got" + +printf '\nbd-audit-ids-test: %d passed, %d failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ]