From 0151df23fbc14c7c36d6a40baf52f1e0b8fae6bd Mon Sep 17 00:00:00 2001 From: Boyd Cohen Date: Sat, 8 Aug 2026 23:22:37 -0600 Subject: [PATCH 1/2] Green must mean measured: assert the reproduction population, not just its failures Run 31289030963 was five green jobs out of seven, and two of those five were skips. verify-reproduction.sh exits 0 when a package's declared version is not published, which is correct and invisible: at job level a skip and a reproduction are the same tick. Coverage had fallen from six packages to five, caused by a CORRECT fix. x402-op-authorize was bumped off the 0.4.0 it had drifted from, which moved the one package this check was written for out of the check's reach. Nothing went wrong and coverage dropped anyway. - verify-reproduction.sh records measured|skipped|inconclusive at every exit path. A dist/ mismatch counts as MEASURED: the comparison ran and the answer was no. A job that dies without writing counts as no result, never as a pass. - assert-reproduction-population.sh asserts three separable things: every package under packages/ reported, the measured count is at or above the declared floor, and the floor itself is readable. Expected population is read from packages/ rather than the matrix, so a package added to the repo and forgotten in the workflow fails here. - scripts/reproduction-floor.txt declares the floor (5) as a committed decision, so accepting less coverage is an edit a reviewer sees rather than a side effect. - Three self-tests: the assertion must fail when everything is skipped (naming the floor it read, not merely exiting non-zero), must fail when a package reports nothing, and must PASS when the floor is met, so the first two are not satisfied by an assertion that always fails. Verified locally: all-skipped exit 1, one-missing exit 1, absent floor exit 1, unparseable floor exit 1, floor met exit 0. --- .github/workflows/reproduction.yml | 82 ++++++++++++++++ scripts/assert-reproduction-population.sh | 113 ++++++++++++++++++++++ scripts/reproduction-floor.txt | 29 ++++++ scripts/verify-reproduction.sh | 39 +++++++- 4 files changed, 258 insertions(+), 5 deletions(-) create mode 100755 scripts/assert-reproduction-population.sh create mode 100644 scripts/reproduction-floor.txt diff --git a/.github/workflows/reproduction.yml b/.github/workflows/reproduction.yml index 99f5e79..0c90193 100644 --- a/.github/workflows/reproduction.yml +++ b/.github/workflows/reproduction.yml @@ -45,8 +45,38 @@ jobs: node-version: 22.22.3 - name: Reproduce ${{ matrix.package }} + env: + REPRO_RESULT_FILE: ${{ runner.temp }}/result.txt run: bash scripts/verify-reproduction.sh packages/${{ matrix.package }} + # if: always() ON PURPOSE. A job that failed still measured something, and a job that died + # without writing anything must be visible as "no result" rather than absent. Uploading only + # on success would make the population job blind to exactly the runs worth counting. + - name: Record the outcome for the population assertion + if: always() + uses: actions/upload-artifact@v4 + with: + name: repro-result-${{ matrix.package }} + path: ${{ runner.temp }}/result.txt + if-no-files-found: ignore + retention-days: 7 + + # ─── HOW MANY PACKAGES WERE ACTUALLY CHECKED? ──────────────────────────────────────────────────── + # The matrix above reports whether each job PASSED. It cannot report how many packages were + # MEASURED, because a skip passes. Measured 2026-08-09: five green jobs, two of them skips. + population: + needs: reproduce + if: always() # the count matters most on the runs where something went red + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + with: + path: results + pattern: repro-result-* + - name: Assert the measured population + run: bash scripts/assert-reproduction-population.sh results + # The publish guard is exercised here too, so its own failure modes stay live rather than being # discovered at publish time by whoever is publishing. guard-self-test: @@ -102,3 +132,55 @@ jobs: exit 1 fi echo "negative control fired, naming the instrument" + + # ─── THE POPULATION ASSERTION MUST BE ABLE TO FAIL ────────────────────────────────────────── + # A coverage check that cannot go red is worth less than no coverage check, because it reads + # as coverage. These cases are synthetic on purpose: they exercise the assertion itself rather + # than the state of this repo today, so the assertion stays honest as the real population moves. + - name: The population assertion must fail when everything is skipped + run: | + FLOOR="$(grep -vE '^\s*#|^\s*$' scripts/reproduction-floor.txt | head -1 | tr -d '[:space:]')" + mkdir -p /tmp/allskip + for d in packages/*/; do + p="$(basename "${d%/}")" + mkdir -p "/tmp/allskip/repro-result-$p" + echo "skipped $p@0.0.0-synthetic" > "/tmp/allskip/repro-result-$p/result.txt" + done + OUT="$(bash scripts/assert-reproduction-population.sh /tmp/allskip 2>&1 || true)" + echo "$OUT" + # ASSERT THE REASON. A bare "it exited non-zero" would also be satisfied by the missing-result + # branch, so this requires the floor comparison specifically, quoting the floor it read. + if ! printf '%s' "$OUT" | grep -q "only 0 package(s) were actually measured against the registry; the floor is ${FLOOR}"; then + echo "::error::a matrix where every package is unpublished did not trip the floor. The coverage check cannot fail, so its green means nothing." + exit 1 + fi + echo "floor tripped at 0 measured, as intended" + + - name: The population assertion must fail when a package reports nothing + run: | + mkdir -p /tmp/missing + FIRST="" + for d in packages/*/; do + p="$(basename "${d%/}")" + if [ -z "$FIRST" ]; then FIRST="$p"; continue; fi # this one deliberately reports nothing + mkdir -p "/tmp/missing/repro-result-$p" + echo "measured $p@0.0.0-synthetic" > "/tmp/missing/repro-result-$p/result.txt" + done + OUT="$(bash scripts/assert-reproduction-population.sh /tmp/missing 2>&1 || true)" + echo "$OUT" + if ! printf '%s' "$OUT" | grep -q "1 package(s) in packages/ produced no reproduction result"; then + echo "::error::a package that reported nothing was not counted as missing — an omitted or crashed job would read as covered" + exit 1 + fi + echo "missing-result branch fired for $FIRST, as intended" + + - name: The population assertion must PASS when the floor is met + run: | + mkdir -p /tmp/ok + for d in packages/*/; do + p="$(basename "${d%/}")" + mkdir -p "/tmp/ok/repro-result-$p" + echo "measured $p@0.0.0-synthetic" > "/tmp/ok/repro-result-$p/result.txt" + done + # Without this the two cases above are satisfied by an assertion that always fails. + bash scripts/assert-reproduction-population.sh /tmp/ok diff --git a/scripts/assert-reproduction-population.sh b/scripts/assert-reproduction-population.sh new file mode 100755 index 0000000..88aa0f7 --- /dev/null +++ b/scripts/assert-reproduction-population.sh @@ -0,0 +1,113 @@ +#!/bin/bash +# HOW MANY PACKAGES WERE ACTUALLY MEASURED? Asserted here, because the matrix cannot answer it. +# +# The reproduction matrix reports whether each job PASSED. It has never reported how many packages +# were CHECKED, and those are different facts: a skip passes. Measured 2026-08-09, five of seven +# jobs were green and two of those were skips, so "7 jobs, 0 failures" described a run that had +# compared five packages against the registry and two against nothing. +# +# This asserts three things, each able to fail on its own: +# +# 1. EVERY package directory produced a result. A job that was omitted from the matrix, or that +# died before it could record anything, is counted as "no result" rather than passed. The +# expected population is read from packages/ rather than from the matrix, so a package added +# to the repo and forgotten in the workflow fails here instead of being silently uncovered. +# 2. The measured count is at or above the floor declared in scripts/reproduction-floor.txt. +# 3. The floor file itself is present and parseable. Fail closed: an unreadable floor is not +# permission to skip the check, and a missing file must not read as a floor of zero. +# +# Usage: assert-reproduction-population.sh +# holds one file per package, each containing " @", +# where is measured | skipped | inconclusive. +set -uo pipefail + +DIR="${1:?usage: assert-reproduction-population.sh }" +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +FLOOR_FILE="${ROOT}/scripts/reproduction-floor.txt" + +# ─── 3. THE FLOOR MUST BE READABLE ─────────────────────────────────────────────────────────────── +if [ ! -f "${FLOOR_FILE}" ]; then + echo "REFUSING: ${FLOOR_FILE} is missing. A floor that cannot be read is not a floor of zero." >&2 + exit 1 +fi +FLOOR="$(grep -vE '^\s*#|^\s*$' "${FLOOR_FILE}" | head -1 | tr -d '[:space:]')" +if ! printf '%s' "${FLOOR}" | grep -qE '^[0-9]+$'; then + echo "REFUSING: ${FLOOR_FILE} does not contain a bare integer (read: '${FLOOR}')." >&2 + exit 1 +fi + +# ─── 1. EVERY PACKAGE MUST HAVE REPORTED ───────────────────────────────────────────────────────── +EXPECTED=() +for d in "${ROOT}"/packages/*/; do + [ -f "${d}package.json" ] || continue + EXPECTED+=("$(basename "${d%/}")") +done +if [ "${#EXPECTED[@]}" -eq 0 ]; then + echo "REFUSING: no package directories found under ${ROOT}/packages. An empty population passes" >&2 + echo " every check trivially, which is the failure this line exists to prevent." >&2 + exit 1 +fi + +MEASURED=0; SKIPPED=0; INCONCLUSIVE=0; MISSING=0 +echo "REPRODUCTION POPULATION" +echo +for pkg in "${EXPECTED[@]}"; do + # The result file may arrive at //result.txt or /repro-result-/result.txt + # depending on how artifacts were laid down. Find it rather than assuming one shape. + f="$(find "${DIR}" -type f -path "*${pkg}*" -name '*.txt' 2>/dev/null | head -1)" + if [ -z "${f}" ] || [ ! -s "${f}" ]; then + printf ' %-26s NO RESULT\n' "${pkg}" + MISSING=$((MISSING + 1)); continue + fi + line="$(head -1 "${f}")" + state="${line%% *}" + case "${state}" in + measured) MEASURED=$((MEASURED + 1)) ;; + skipped) SKIPPED=$((SKIPPED + 1)) ;; + inconclusive) INCONCLUSIVE=$((INCONCLUSIVE + 1)) ;; + *) printf ' %-26s UNRECOGNISED STATE: %s\n' "${pkg}" "${line}"; MISSING=$((MISSING + 1)); continue ;; + esac + printf ' %-26s %s\n' "${pkg}" "${line}" +done + +echo +echo " packages in repo : ${#EXPECTED[@]}" +echo " measured : ${MEASURED} (floor ${FLOOR})" +echo " skipped : ${SKIPPED}" +echo " inconclusive : ${INCONCLUSIVE}" +echo " no result : ${MISSING}" +echo + +FAILED=0 + +if [ "${MISSING}" -gt 0 ]; then + cat >&2 <&2 < "${REPRO_RESULT_FILE}" + fi + return 0 +} + +cd "$PKG_DIR" || { record inconclusive; exit 1; } NAME="$(node -p 'require("./package.json").name')" VER="$(node -p 'require("./package.json").version')" @@ -25,22 +49,24 @@ VER="$(node -p 'require("./package.json").version')" OUT="$(npm view "${NAME}@${VER}" version --json 2>/dev/null)"; RC=$? if [ $RC -ne 0 ] && printf '%s' "$OUT" | grep -q '"code": *"E404"'; then echo "SKIP ${NAME}@${VER} — not published, nothing to reproduce" + record skipped exit 0 fi if [ $RC -ne 0 ] || ! printf '%s' "$OUT" | grep -q "\"${VER}\""; then echo "FAIL ${NAME}@${VER} — could not establish whether this version is published (fail-closed)" >&2 + record inconclusive exit 1 fi WORK="$(mktemp -d)"; trap 'rm -rf "$WORK"' EXIT ( cd "$WORK" && npm pack "${NAME}@${VER}" >/dev/null 2>&1 ) || { - echo "FAIL ${NAME}@${VER} — could not fetch the published tarball (fail-closed)" >&2; exit 1; } + echo "FAIL ${NAME}@${VER} — could not fetch the published tarball (fail-closed)" >&2; record inconclusive; exit 1; } PUBLISHED="$(ls "$WORK"/*.tgz | head -1)" -npm ci --silent >/dev/null 2>&1 || { echo "FAIL ${NAME}@${VER} — npm ci failed" >&2; exit 1; } -npm run build --silent >/dev/null 2>&1 || { echo "FAIL ${NAME}@${VER} — build failed" >&2; exit 1; } +npm ci --silent >/dev/null 2>&1 || { echo "FAIL ${NAME}@${VER} — npm ci failed" >&2; record inconclusive; exit 1; } +npm run build --silent >/dev/null 2>&1 || { echo "FAIL ${NAME}@${VER} — build failed" >&2; record inconclusive; exit 1; } REBUILT="$(npm pack --silent 2>/dev/null | tail -1)" -[ -f "$REBUILT" ] || { echo "FAIL ${NAME}@${VER} — npm pack produced nothing" >&2; exit 1; } +[ -f "$REBUILT" ] || { echo "FAIL ${NAME}@${VER} — npm pack produced nothing" >&2; record inconclusive; exit 1; } # WHAT IS COMPARED, AND WHY IT IS NOT THE WHOLE TARBALL. # @@ -67,6 +93,8 @@ if ! diff -rq "$WORK/a/package/dist" "$WORK/b/package/dist" >/dev/null 2>&1; the echo " restore the tree." >&2 diff -rq "$WORK/a/package/dist" "$WORK/b/package/dist" 2>&1 \ | sed "s|$WORK/a/package/dist|published:|g; s|$WORK/b/package/dist|rebuilt:|g" | sed 's/^/ /' >&2 + # MEASURED, not inconclusive: the comparison ran and returned an answer. The answer is "no". + record measured exit 1 fi @@ -80,4 +108,5 @@ if [ -n "$OTHER" ]; then else echo "OK ${NAME}@${VER} — reproduces the published tarball byte-for-byte" fi +record measured exit 0 From 0421051bc7c21bf83b83d0c9ddd0cbceb66101eb Mon Sep 17 00:00:00 2001 From: Boyd Cohen Date: Sat, 8 Aug 2026 23:30:26 -0600 Subject: [PATCH 2/2] The guard self-tests were running against main, not against the pull request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by this PR going red on run 31296563830 with "scripts/reproduction-floor.txt: No such file or directory" — a file this branch adds and main does not have. The job's setup step ran `git checkout -B guard-selftest origin/main`, which it did to give HEAD an upstream so the guard reaches the checks under test. It also replaced the working tree. Workflow STEPS come from the pull request; the scripts/ they exercised came from main. This is invisible while a PR leaves the guard alone, and it inverts the moment one does not: a pull request that BROKE refuse-dirty-publish.sh would have its self-tests pass, because they would be exercising the unbroken copy on main. The control was green for a reason unrelated to the code under review. It only surfaced here because this branch adds a file rather than changing one, turning a silent pass into a missing-file error. - check out the head of the branch under test, which gives HEAD an upstream AND keeps the tree being reviewed. On a push to main the two are the same commit. - assert that the checked-out SHA is the one under review, so the next edit to that step cannot reintroduce this quietly. --- .github/workflows/reproduction.yml | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/.github/workflows/reproduction.yml b/.github/workflows/reproduction.yml index 0c90193..6ff90b6 100644 --- a/.github/workflows/reproduction.yml +++ b/.github/workflows/reproduction.yml @@ -90,11 +90,40 @@ jobs: # The guard refuses a branch with no upstream BEFORE it reaches any later check, and # actions/checkout does not configure upstream tracking. Without this the self-tests below # would go red for the wrong reason and read as "the guard is broken". - - name: Give HEAD an upstream, so the guard reaches the checks under test + # + # ─── THIS STEP USED TO CHECK OUT origin/main, WHICH TESTED THE WRONG TREE ──────────────────── + # Measured 2026-08-09 on run 31296563830. The workflow STEPS come from the pull request, but + # `git checkout -B guard-selftest origin/main` replaced the working tree with main's, so every + # assertion below ran against main's copy of scripts/. The steps were the PR's and the code + # under test was not. + # + # That is invisible while a PR does not touch the guard, and it inverts the moment one does: + # a pull request that BROKE refuse-dirty-publish.sh would have its self-tests pass, because + # they would be exercising the unbroken copy on main. It surfaced only because this PR added + # a file that main does not have, so the step failed with "No such file or directory" instead + # of quietly passing. + # + # Checking out the head of the branch under test gives HEAD an upstream AND keeps the tree + # that is actually being reviewed. On a push to main the two are the same thing. + - name: Give HEAD an upstream, WITHOUT discarding the tree under test run: | - git fetch origin main - git checkout -B guard-selftest origin/main - git branch --set-upstream-to=origin/main guard-selftest + BRANCH="${{ github.head_ref || github.ref_name }}" + git fetch origin "$BRANCH" + git checkout -B guard-selftest "origin/$BRANCH" + git branch --set-upstream-to="origin/$BRANCH" guard-selftest + echo "self-tests are running against origin/$BRANCH at $(git rev-parse --short HEAD)" + + # The tree under test must be THIS branch's, not whatever the previous step happened to leave. + # Without this line the failure above is silent again the next time someone edits that step. + - name: The checked-out tree must be the one under review + run: | + WANT="${{ github.event.pull_request.head.sha || github.sha }}" + GOT="$(git rev-parse HEAD)" + if [ "$WANT" != "$GOT" ]; then + echo "::error::self-tests are running against $GOT but the tree under review is $WANT — they would be testing the wrong code" + exit 1 + fi + echo "tree under test confirmed: $GOT" - name: The guard must refuse an already-published version FOR THAT REASON working-directory: packages/x402-op-authorize