Skip to content

fix(package): close the vacuous pass in the release gate (#98 landed only 1 of 4 commits) - #101

Merged
JC-000 merged 3 commits into
masterfrom
fix/package-verify-vacuous-pass
Aug 14, 2026
Merged

fix(package): close the vacuous pass in the release gate (#98 landed only 1 of 4 commits)#101
JC-000 merged 3 commits into
masterfrom
fix/package-verify-vacuous-pass

Conversation

@JC-000

@JC-000JC-000 commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Recovers three commits that #98 was supposed to carry.#98 merged with only its first commit (c1bc429, partial-matrix handling); the actual fix, its tests, and the preflight wiring never reached master.

(Correction to an earlier draft of this description: the three commits were pushed, not stranded unpushed — origin/fix/package-partial-failure carried all four at 87f501b. They were pushed to the branch after the PR was opened and arrived after the merge at 15:00:33Z, so gh pr view 98 --json commits shows the merge saw only c1bc429. The distinction matters for the lesson: every individual git push reported success, which is exactly why "pushed" got mistaken for "merged" in the sign-off.)

Verified on origin/master before filing:

verify_release.py:120 return sorted(DIST.glob("*.d64")) <- unchanged
expected_d64_images() absent
summarize() absent
tools/test_package_verify.py MISSING

This is worse than a fix that plainly didn't land, because the half that did land is the reporting half. make package-verify now presents as more trustworthy than before while the hole underneath is untouched, and #98's title states the problem is fixed. The new BLOCKER section also masks the defect whenever the build record has FAILED rows, so a casual run looks healthy.

Reproduced on master with the disk checks isolated — build record rewritten all-OK, no artifacts present, SKIP_LISTENER=1:

0/0 checks passed
RELEASE ARTIFACTS VERIFIED
exit 0

An all-OK record, zero artifacts, green gate.

The three commits

  1. 061f0ab — coverage derived from the build record rather than glob("*.d64"), so absent images fail instead of contributing zero checks; any zero-check run fails; and the word VERIFIED is reserved for runs where every section actually executed.
  2. 2b02390tools/test_package_verify.py, 31 assertions pinning the verdict logic, no VICE and no builds required. summarize() was made pure so the verdict is testable at all.
  3. 68c61c8 — those tests wired as a preflight to make package-verify, because a regression test nobody runs is the same defect class it guards against.

Verified here

python3 tools/test_package_verify.py -> 31 passed, 0 failed

The underlying defect is worth stating plainly, since it is subtler than "the gate passes vacuously": the disk checks globbed while the listener check asserted, so whether an empty run read green depended on which style the one surviving check happened to use. A correctness property riding on an unrelated check's defensive style stays invisible until that check is skipped — precisely when someone is narrowing a failure and least likely to be suspicious.

Found by the certs lane, which noticed a sign-off saying "#98 open with four commits" disagreed with a query saying MERGED, and resolved the mismatch by measuring rather than assuming one side was stale.

🤖 Generated with Claude Code

JC-000and others added 3 commits August 14, 2026 10:17
…t not pass
LaneCerts found the sharper version of the bug I was fixing: at master,
`SKIP_REBUILD=1 SKIP_VICE=1 make package-verify` printed
2/2 checks passed
RELEASE ARTIFACTS VERIFIED
on a dist/ with half its PRGs missing and no disk images or manifest at
all. Reproduced here, and it is worse than reported — with the build
record present but dist/ emptied, the gate printed
0/0 checks passed
RELEASE ARTIFACTS VERIFIED exit 0
A gate that verified literally nothing reported success. My earlier
commit on this branch did not close it: it only caught variants that
FAILED to build, not artifacts that were never produced.
Cause: both disk checks iterated `DIST.glob("*.d64")`. No disks meant no
iterations, no records, nothing failed. Absence of evidence read as
evidence of correctness — which is the exact failure mode this repo's
own guidance warns about.
- expected_d64_images() derives the images that MUST exist from the
build record (one per built variant, one per backend). Both disk
checks iterate that, and record a FAIL for any expected image that
is absent, plus one for stray images no variant accounts for.
build_prgs.sh now records `backend=` per variant so this derivation
needs no second copy of the matrix.
- A run that records zero checks fails outright.
- "RELEASE ARTIFACTS VERIFIED" is now reserved for a run where every
section executed. Any SKIP_* yields "PARTIAL VERIFICATION — ... Not
a release gate", still exit 0 so SKIP_* stays usable for narrowing.
Measured, all four cases:
A built PRGs, disks deleted 0/4, four "expected but absent", exit 1
B make package at the broken pin partial artifacts + blocker, exit 1
C partial dist, disks present 6/6 present verify, 2 variants
missing, RELEASE INCOMPLETE, exit 1
D all-OK record, SKIP_* set 4/4, PARTIAL VERIFICATION, exit 0
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds tools/test_package_verify.py — 31 assertions over the gate's verdict
logic and its coverage derivation. No VICE, no builds, milliseconds.
summarize() is split out of main() to make it testable, and because the
ordering inside it IS the contract: zero checks fails before the friendly
skip branch can rescue it, failures outrank the missing-variant note, and
"RELEASE ARTIFACTS VERIFIED" is reachable only from a complete run.
The case guarded hardest is SKIP_* with everything passing (suggested by
LaneCerts): it is the invocation people actually use while iterating, so
it is where the pressure to just say VERIFIED will come back. It must
stay exit 0 to remain usable while never producing the word VERIFIED.
The subtlest test is that skipping every section on an empty dist/ must
NOT downgrade to a cheerful PARTIAL — that is the original bug wearing a
different hat.
Also corrects an overstatement in this branch's PR description. On a
genuinely clean tree master's gate does not go green: with no listener
bundle it reports 0/1 exit 1, because check_listener asserts the
bundle's presence explicitly rather than iterating. The vacuous pass
needed the bundle to be present:
no disks, no bundle SKIP_REBUILD SKIP_VICE 0/1 exit 1
no disks, bundle present SKIP_REBUILD SKIP_VICE 2/2 VERIFIED exit 0
no disks, bundle present + SKIP_LISTENER 0/0 VERIFIED exit 0
The real defect is that inconsistency — the disk checks were globs, the
listener check was an assertion, and the gate's correctness rode on
which style each check happened to use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A regression test nobody runs is not a guard. tools/test_package_verify.py
now runs as a preflight to `make package-verify`.
The ordering is the point: verify_release.py shipped with a bug where it
reported RELEASE ARTIFACTS VERIFIED having checked nothing, so "the gate
said yes" is only worth something if the gate's yes still means what it
should. A broken verdict function now stops the run instead of blessing
a release. Costs milliseconds; needs no VICE and no build.
Not added to tools/run_all_tests.py: that runner allocates a VICE
instance per suite and dispatches run_tests(transport, labels, seed), a
shape a pure-logic test does not fit. The gate is the right home.
Measured: preflight 31 passed, then the gate reports RELEASE INCOMPLETE
against the current broken pin, exit 1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JC-000
JC-000 merged commit 021e470 into masterAug 14, 2026
@JC-000

Copy link
Copy Markdown
OwnerAuthor

Independent confirmation from the packaging lane, plus one thing to check at merge time.

I arrived here from the other direction — pushed this branch, tried to open a PR, and found #101 already filed with exactly these three commits (061f0ab, 2b02390, 68c61c8). Same commits, same fix. Nothing to reconcile.

I made one factual correction to the description above: the commits were pushed, not stranded unpushed. origin/fix/package-partial-failure carried all four at 87f501b; they simply arrived after #98 merged. That distinction is the actual lesson — every git push reported success, so "pushed" got mistaken for "merged" in a sign-off that nobody had reason to doubt.

Reproduced on merged master before applying anything

At 660101a, disk checks isolated (both result=FAILED rows rewritten to OK so the BLOCKER section has nothing to say, no disk images present):

$ make package-verify SKIP_REBUILD=1 SKIP_VICE=1 SKIP_LISTENER=1
Verifying 4 PRG variants and 0 disk images in …/dist
=== 2a. D64 contents (c1541 read-back, byte-compare) ===
============================================================
0/0 checks passed
RELEASE ARTIFACTS VERIFIED
make exit=0

An all-OK build record, zero artifacts on disk, green gate, exit 0.

Identical setup with this PR applied:

0/6 checks passed
FAILED:
- c64-https-ip65-onchip.d64 exists
- c64-https-ip65-reu.d64 exists
- c64-https-ip65.d64 exists
- c64-https-uci-onchip.d64 exists
- c64-https-uci-reu.d64 exists
- c64-https-uci.d64 exists
make: *** [package-verify] Error 1

Preflight: 31 passed, 0 failed.

Please verify the merge carries all three

This is the whole reason the PR exists, so it would be an unusually bad one to repeat:

git log --oneline origin/master | head -4 # expect 061f0ab, 2b02390, 68c61c8
git cat-file -e origin/master:tools/test_package_verify.py &&echo present
git show origin/master:tools/package/verify_release.py | grep -c expected_d64_images

If the last two don't come back clean, the merge dropped commits again.

Unrelated blocker, still open

src/lib_contract_asserts.s:166 hardcodes LIB_NISTCURVES_SHARED_PRIMITIVES = $0007; the onchip archive correctly reports $0005 at the v0.9.1 pin (od65 on lib_manifest_p256verify_onchip.o; matches the FP_ONCHIP_MUL row in libs/nistcurves/CHANGELOG.md), so both onchip variants fail to link. #94's file. The release cannot be tagged until that lands, independently of this PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@JC-000