Merge pull request #226 from fiddlerwoaroof/tooling/52eq-audit
tooling: the dead-ID audit compares positively and is a checked script (52eq)
This commit is contained in:
Ed Langley
GitHub
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
Executable
+75
@@ -0,0 +1,75 @@
|
||||
#!/usr/bin/env bash
|
||||
# The dead-ID audit: every improvise-<id> 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 ' ')"
|
||||
Executable
+87
@@ -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 ]
|
||||
Reference in New Issue
Block a user