Skip to content

Commit 9838648

Browse files
noahgiftclaude
noahgift
and
claude
authored
fix(tooling): a stale apr binary can no longer be resolved, hardcoded, or dogfooded (#2357)
Four `apr` binaries were found coexisting on the dev box: ~/.local/bin/apr 0.60.0 (no SHA) 26 days ~/.cargo/bin/apr 0.62.0 (3418754) FRESH /mnt/nvme-raid0/coverage/aprender/release/apr 0.61.0 (9514558) 1 day /mnt/nvme-raid0/targets/aprender/release/apr 0.60.0 (c55a333) 6 days This class has now caused damage three times: qwen-story-daily validated 24-day-old code while reporting green (#2344); a v0.62.0 release smoke-test read a five-hour-old binary and reported a meaningless pass; and a real product regression (#2350) stayed hidden 24 days because the nightly ran a pre-regression binary. Detection existed and was not enough. 1. RESOLUTION: ask cargo, never search. apr_bin.sh derived $APR from $CARGO_HOME/bin then PATH — both of which SEARCH, and a search finds the wrong binary eventually. It now asks `cargo metadata` for this checkout's target_directory. That matters because `.cargo/config.toml` redirects target-dir and is GITIGNORED (.gitignore:55) while its siblings .cargo/audit.toml and .cargo/mutants.toml are tracked — the directory looks version-controlled, the file that moves every build output is not. Measured: main checkout -> /mnt/nvme-raid0/coverage/aprender fresh worktree -> <worktree>/target So no absolute path is correct in both. $CARGO_HOME/bin is kept LAST (cargo install leaves nothing in target/), still freshness-checked. 2. FAIL CLOSED: `APR_BIN_STRICT=1` makes "not a git checkout" a refusal. It previously returned 0 with "freshness not asserted" — a fail-OPEN hole in a script whose only job is refusing unproven binaries. 3. HARDCODED ABSOLUTE PATHS are now a build failure. check_apr_bin_pinned.sh treated `/mnt/.../target/release/apr` as PINNED because it ends in target/release/apr — so the exact path that was 6 days and two minor versions stale passed the guard. New ABS-APR class, mutation-verified by injecting that literal path into qwen-story.sh: RED exit 1, GREEN exit 0 after revert. The regex is anchored at the path START; the first draft matched the `/apr` inside RELATIVE `target/release/apr` and flagged correct code. Verified against a 12-case table (4 must match, 8 must not) — this regex class has now been gotten wrong four times here, so the table ships with it. 4. THE DOGFOOD PROTOCOL certified releases with an unpinned binary: - Gate 1 ran `cargo install --path` (writes $CARGO_HOME/bin) and then a BARE `apr`, which PATH resolved to the 26-day-old ~/.local/bin copy. - Gate 13, the protocol's ONLY freshness assertion, SKIPPED when the binary had no embedded SHA "(likely crates.io install)". `~/.local/bin/apr` reports exactly `apr 0.60.0 (v0.60.0+no-git)`. The one artifact most likely to be executed was the one that made the only gate excuse itself. Both now go through `. scripts/apr_bin.sh || exit 1`; no-SHA is a FAIL unless DOGFOOD_ALLOW_UNPINNED=1 is set deliberately. 5. CLAUDE.md's "MANDATORY: use apr tools first" block — the most-followed instruction in the repo — told every agent to run bare `apr qa`. It now pins Step 0 and uses "$APR" throughout. Guards green: check_apr_bin_pinned, check_sourced_libs_option_neutral, check_pass_grep_anchored. bashrs errors across both scripts: 8 -> 5. NOT fixed here: the stale binaries themselves still exist on the box (deleting another user's ~/.local/bin is an operator decision, not a repo change), and historical specs under docs/specifications/ still quote the old absolute path as a record of past runs. Both tracked separately. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 3418754 commit 9838648

4 files changed

Lines changed: 147 additions & 13 deletions

File tree

‎.claude/skills/dogfood/SKILL.md‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,24 @@ Run ALL gates below. For each gate, run the check, report PASS/FAIL/SKIP with ev
4141

4242
```bash
4343
cargo install --path crates/apr-cli --force 2>&1 | tail -5
44-
apr --version
44+
# Resolve the binary through the guard, NEVER a bare `apr`. Sourcing exports
45+
# $APR and hard-fails when it was not built from HEAD.
46+
. scripts/apr_bin.sh || exit 1
47+
APR_BIN_STRICT=1 "$APR" --version
4548
git rev-parse --short HEAD
4649
```
4750

4851
PASS if version string contains the HEAD commit hash. FAIL if build errors or mismatch.
4952

53+
> **Why `$APR` and not `apr`.** The previous version of this gate installed to
54+
> `$CARGO_HOME/bin` and then invoked a **bare `apr`**, which PATH resolves
55+
> independently. On the machine that runs this protocol `~/.local/bin/apr`
56+
> preceded `~/.cargo/bin/apr` and held a 26-day-old 0.60.0 build, so Gate 1
57+
> installed a fresh binary and then version-checked a stale one. That is the
58+
> exact defect #2344 fixed in `qwen-story-daily`, and it survived here — in the
59+
> protocol that certifies releases. A dogfood run is worthless if it exercised a
60+
> binary other than the one being shipped.
61+
5062
## Gate 2: Full Command Grid (FALSIFY-QA-001, FALSIFY-QA-009)
5163

5264
Auto-discover models:
@@ -538,16 +550,29 @@ reporting a stale commit hash in git worktrees because `build.rs` watches a
538550
hardcoded `../../.git/HEAD` path that doesn't exist in a worktree layout.
539551

540552
```bash
541-
# After cargo install, apr --version SHA MUST match git rev-parse --short HEAD.
553+
# After cargo install, the SHA MUST match git rev-parse --short HEAD.
542554
# Run this from inside the source checkout (or worktree) you just built from.
543-
APR_SHA=$(apr --version 2>&1 | grep -oE '\([a-f0-9]{7,}\)' | tr -d '()')
555+
. scripts/apr_bin.sh || exit 1
556+
APR_SHA=$("$APR" --version 2>&1 | grep -oE '\([a-f0-9]{7,}\)' | tr -d '()')
544557
HEAD_SHA=$(git rev-parse --short HEAD)
545558
if [ -n "$APR_SHA" ] && [ "$APR_SHA" = "$HEAD_SHA" ]; then
546-
echo "G13 PASS: apr --version SHA ($APR_SHA) matches HEAD"
559+
echo "G13 PASS: $APR SHA ($APR_SHA) matches HEAD"
547560
elif [ -z "$APR_SHA" ]; then
548-
echo "G13 SKIP: apr --version has no embedded SHA (likely crates.io install)"
561+
# NOT a SKIP. A binary with no embedded SHA cannot be shown to be the code
562+
# under test, and "cannot prove" must never read as "fine". This branch used
563+
# to SKIP for "likely crates.io install" — and the binary that wins PATH
564+
# resolution on this machine is `~/.local/bin/apr`, which reports exactly
565+
# `apr 0.60.0 (v0.60.0+no-git)`. So the one stale artifact most likely to be
566+
# executed was also the one that made the protocol's ONLY freshness gate
567+
# excuse itself. If you are deliberately dogfooding a published crates.io
568+
# build, say so explicitly with DOGFOOD_ALLOW_UNPINNED=1.
569+
if [ "${DOGFOOD_ALLOW_UNPINNED:-0}" = "1" ]; then
570+
echo "G13 SKIP (explicitly allowed): $APR has no embedded SHA"
571+
else
572+
echo "G13 FAIL: $APR has NO embedded SHA — cannot prove it is HEAD. Set DOGFOOD_ALLOW_UNPINNED=1 only for a deliberate crates.io dogfood."
573+
fi
549574
else
550-
echo "G13 FAIL: apr --version SHA=$APR_SHA but HEAD=$HEAD_SHA (#1862)"
575+
echo "G13 FAIL: $APR SHA=$APR_SHA but HEAD=$HEAD_SHA (#1862)"
551576
fi
552577
```
553578

‎CLAUDE.md‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,33 @@ make coverage # Coverage report (enforced floor 88%, target ≥95
8484

8585
GH-202 lesson: we read code instead of running `apr qa` which would have instantly shown the failure.
8686

87+
**Step 0 — pin the binary, ALWAYS.** Never invoke a bare `apr`, and never hardcode
88+
an absolute path to one. Four `apr` binaries were found coexisting on the dev box
89+
(0.60.0 ×2, 0.61.0, 0.62.0); a bare `apr` resolved to a **26-day-old** copy, and
90+
the path this file used to call "canonical" was two minor versions stale. There is
91+
no correct path to hardcode — `.cargo/config.toml` redirects cargo's target-dir and
92+
is gitignored, so the main checkout and a fresh worktree build to different places.
93+
94+
```bash
95+
. scripts/apr_bin.sh || exit 1 # exports $APR, proves it was built from HEAD
96+
```
97+
98+
Everything below uses `"$APR"`. A diagnostic run against the wrong binary is worse
99+
than no diagnostic: it produces a confident answer about code you are not running.
100+
87101
```bash
88102
# Step 1: ALWAYS start here (catches 80% of issues)
89-
apr qa model.apr
103+
"$APR" qa model.apr
90104

91105
# Step 2: Check tensor shapes/stats
92-
apr tensors model.apr | head -20
106+
"$APR" tensors model.apr | head -20
93107

94108
# Step 3: Diff against known-good model
95-
apr diff model.apr reference.gguf
109+
"$APR" diff model.apr reference.gguf
96110

97111
# Step 4: Format/metadata integrity
98-
apr validate model.apr --quality
99-
apr lint model.apr
112+
"$APR" validate model.apr --quality
113+
"$APR" lint model.apr
100114

101115
# Step 5: ONLY NOW read code
102116
```

‎scripts/apr_bin.sh‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,63 @@ apr_bin_die() {
3838
return 1
3939
}
4040

41-
# Resolve the intended binary, most explicit first.
41+
# Resolve the binary THIS CHECKOUT builds. Ask cargo; never guess.
42+
#
43+
# This deliberately does NOT consult PATH, $CARGO_HOME/bin, or any absolute
44+
# path. Four `apr` binaries were found coexisting on one machine - 0.60.0,
45+
# 0.61.0, 0.62.0 and a second 0.60.0 with no embedded SHA at all - and every
46+
# resolution strategy that *searches* for a binary will eventually find the
47+
# wrong one. Detection was already in place and still lost 24 days to a stale
48+
# nightly, so the fix is to stop searching.
49+
#
50+
# `cargo metadata` reports the real target directory under every checkout shape,
51+
# which matters because `.cargo/config.toml` redirects it and is GITIGNORED
52+
# (.gitignore:55) while its siblings `.cargo/audit.toml` and `.cargo/mutants.toml`
53+
# are tracked. The directory looks version-controlled; the file that moves every
54+
# build output is not. Measured:
55+
# main checkout -> /mnt/nvme-raid0/coverage/aprender (redirect applies)
56+
# fresh worktree -> <worktree>/target (no config, cargo default)
57+
# A hardcoded absolute path is therefore right in exactly one of those and
58+
# silently wrong in the other - which is how a release smoke-test came to read a
59+
# five-hour-old binary and report a meaningless pass.
60+
apr_bin_target_dir() {
61+
local here
62+
here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)
63+
(cd "$here" && cargo metadata --no-deps --format-version 1 2>/dev/null) \
64+
| jq -r '.target_directory // empty' 2>/dev/null
65+
}
66+
4267
apr_bin_resolve() {
68+
# Explicit override is the ONLY escape hatch - needed for A/B work such as
69+
# comparing a released binary against HEAD. It is still freshness-checked
70+
# below, so it cannot be used to smuggle a stale binary past the gate.
4371
if [ -n "${APR_BIN:-}" ]; then
4472
printf '%s\n' "$APR_BIN"
4573
return 0
4674
fi
75+
local td
76+
td=$(apr_bin_target_dir)
77+
[ -n "$td" ] || return 1
78+
if [ -x "$td/release/apr" ]; then
79+
printf '%s\n' "$td/release/apr"
80+
return 0
81+
fi
82+
if [ -x "$td/debug/apr" ]; then
83+
printf '%s\n' "$td/debug/apr"
84+
return 0
85+
fi
86+
# Last resort: the `cargo install` destination. `cargo install` builds in a
87+
# temp dir and copies only the finished binary here, so it leaves nothing in
88+
# the target dir above - qwen-story-daily installs exactly this way. Kept
89+
# LAST so a checkout that has built its own binary always tests that one,
90+
# and it is still freshness-checked like every other candidate, so this is a
91+
# fallback in resolution order only, never a way around the gate.
4792
local cargo_home="${CARGO_HOME:-$HOME/.cargo}"
4893
if [ -x "$cargo_home/bin/apr" ]; then
4994
printf '%s\n' "$cargo_home/bin/apr"
5095
return 0
5196
fi
52-
command -v apr 2>/dev/null || return 1
97+
return 1
5398
}
5499

55100
# Assert the binary's embedded SHA matches HEAD. No-op outside a git checkout
@@ -60,6 +105,17 @@ apr_bin_assert_fresh() {
60105

61106
reported=$("$bin" --version 2>&1 || true)
62107

108+
# FAIL CLOSED outside a git checkout when strict. The old behaviour returned
109+
# 0 here ("freshness not asserted"), which meant any binary passed the guard
110+
# as long as you ran it from the wrong directory - a fail-OPEN hole in a
111+
# script whose entire job is to refuse unproven binaries. Release and
112+
# dogfood surfaces set APR_BIN_STRICT=1 so "cannot prove" means "refuse".
113+
if [ "${APR_BIN_STRICT:-0}" = "1" ] && ! git rev-parse --short HEAD >/dev/null 2>&1; then
114+
printf 'apr_bin: STRICT mode and this is not a git checkout - cannot prove %s\n' "$bin" >&2
115+
printf ' was built from the code under test, so it is refused.\n' >&2
116+
return 1
117+
fi
118+
63119
if ! git rev-parse --short HEAD >/dev/null 2>&1; then
64120
printf 'apr_bin: %s (%s) - not a git checkout, freshness not asserted\n' \
65121
"$bin" "$reported" >&2

‎scripts/check_apr_bin_pinned.sh‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ scanned=0
5656
# by the case table below, not by reading.
5757
BARE_APR='(^|[;&|]|&&|\|\||run:)[[:space:]]*apr[[:space:]]+[a-z]'
5858

59+
# An absolute path whose last component is `apr`. Anchored on a leading `/`,
60+
# `~/` or `$HOME/` so relative `target/release/apr` (correct inside a checkout)
61+
# is untouched, and requiring the path to END at `apr` so `apr-cli`,
62+
# `aprender-*` and `.../apr_bin.sh` do not match.
63+
# The leading anchor is load-bearing and was wrong in the first draft: without
64+
# it, `[A-Za-z0-9_.$/-]*` happily matched the `/apr` inside RELATIVE
65+
# `target/release/apr`, flagging correct code. Verified against a 12-case table
66+
# (4 absolute forms must match, 8 relative/`$APR`/`--bin apr`/prose forms must
67+
# not). This regex class has now been gotten wrong four times in this repo; if
68+
# you change it, re-run the table rather than reading it.
69+
ABS_APR='(^|[[:space:]"'"'"'=(])(/|~/|\$HOME/)[A-Za-z0-9_.$/-]*/apr([[:space:]"'"'"']|$)'
70+
5971
check_file() {
6072
local f="$1" n=0
6173
scanned=$((scanned + 1))
@@ -91,6 +103,33 @@ check_file() {
91103
printf ' %s\n' "$trimmed"
92104
n=$((n + 1))
93105
done < <(grep -nE "$BARE_APR" "$f" 2>/dev/null || true)
106+
107+
# SECOND CLASS: an ABSOLUTE hardcoded apr path.
108+
#
109+
# This is the other half of the same defect, and the `case` above would wave
110+
# it straight through: `/mnt/nvme-raid0/targets/aprender/release/apr` ends in
111+
# `target/release/apr`, so it matched the "already pinned" list. It is not
112+
# pinned to anything - it names one machine's build output, which on
113+
# 2026-08-01 was 6 days and TWO MINOR VERSIONS stale while docs still called
114+
# it canonical. A release smoke-test read it and reported a meaningless pass.
115+
#
116+
# There is no correct absolute path to hardcode: `.cargo/config.toml`
117+
# redirects cargo's target-dir and is gitignored, so the main checkout builds
118+
# to /mnt/nvme-raid0/coverage/aprender while a fresh worktree builds to
119+
# <worktree>/target. Any absolute path is right in one and silently wrong in
120+
# the other. Use `. scripts/apr_bin.sh || exit 1`, which asks cargo.
121+
while IFS= read -r hit; do
122+
local lineno text trimmed
123+
lineno="${hit%%:*}"
124+
text="${hit#*:}"
125+
trimmed=$(printf '%s' "$text" | sed 's/^[[:space:]]*//')
126+
case "$trimmed" in '#'*) continue ;; *) ;; esac
127+
# apr_bin.sh itself documents these paths in its own comments.
128+
case "$f" in */apr_bin.sh|*/check_apr_bin_pinned.sh) continue ;; *) ;; esac
129+
printf 'ABS-APR %s:%s\n' "$f" "$lineno"
130+
printf ' %s\n' "$trimmed"
131+
n=$((n + 1))
132+
done < <(grep -nE "$ABS_APR" "$f" 2>/dev/null || true)
94133
violations=$((violations + n))
95134
}
96135

0 commit comments

Comments
 (0)