fix(package): partial-matrix handling, and close a gate that could pass having verified nothing - #98
Merged
Merged
Conversation
…f aborting Reported by LaneCerts and reproduced here on a fresh checkout of 21f2ce5: at the nistcurves v0.9.1 pin both onchip variants fail to link, which took the entire packaging pipeline down. `make package` aborted inside build_prgs.sh, so no disk images, no listener and no manifest were ever produced, and `make package-verify` had nothing coherent to report. One broken variant left the operator with no artifacts and no written record of what broke. That is the wrong failure mode for exactly the situation it shows up in. A library bump breaking one profile is common, and the useful outcome is "here are the three that work, here is the error for the fourth" — the release still cannot be cut, but the blocker is legible and the good artifacts are testable. - build_prgs.sh is now three-valued: 0 = all built, 2 = partial, 1 = nothing built. It also greps the first ld65/ca65 diagnostic out of each failed variant's log into dist/build-info.txt, so downstream can state WHY a variant is missing without anyone opening a log. - build_d64.sh skips PRGs that are absent and creates no image when none of its inputs exist, rather than erroring on the first gap. - write_manifest.sh still runs, and opens with an "!! INCOMPLETE RELEASE !!" block naming each missing variant, its make line and its exact error. Missing variants are marked in the guidance list and show FAILED in the size table. A partial release announces itself at the top rather than being a checksum list with fewer lines than it should have. - `make package` runs the pipeline to completion on a partial matrix, then prints the blocker and exits 1. - verify_release.py leads with a BLOCKER section, reports failed variants as [n/a ] in the reproducibility check instead of as fabricated failures, and ends "RELEASE INCOMPLETE" with exit 1. Measured against the real failure at 21f2ce5: make package -> uci-reu + ip65-reu built, both onchip variants recorded with their ld65 error, manifest written with the INCOMPLETE banner, exit 1 make package-verify -> 12/12 present-artifact checks pass (both REU PRGs reproduce byte-for-byte, 4 disk images boot to the correct banner, listener selftest 4/4), 2 variants reported as blockers, exit 1 The underlying link failure is NOT addressed here — it lives in src/lib_contract_asserts.s, which this lane does not own. Diagnosis for whoever picks it up: the assert hardcodes LIB_NISTCURVES_SHARED_PRIMITIVES = $0007, measured at v0.6.0 when both profiles reported that. Upstream has since split ownership from consumption (contract v0.5.0, lib-contract #44), and the FP_ONCHIP_MUL profile now correctly reports $0005 for both masks — od65 on our archives confirms $0007 for the REU build and $0005/$0005 for onchip, exactly matching the table in libs/nistcurves/CHANGELOG.md. The expected value needs to become profile-dependent. Note the assert's own message misdirects: it tells you to re-derive the archive member drops in build_nistcurves_p256.sh, and nothing about the archive surgery changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 14, 2026
JC-000 added a commit
that referenced
this pull request
Aug 14, 2026
fix(package): close the vacuous pass in the release gate (#98 landed only 1 of 4 commits)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #96, prompted by LaneCerts hitting this while running
make packagefor their acceptance.Two bugs, the second much worse than the first.
Problem 1 — one broken variant took the whole pipeline down
At the nistcurves v0.9.1 pin now on master, both onchip variants fail to link.
That took the entire packaging pipeline down:
make packageaborted insidebuild_prgs.sh, so no disk images, no listener and no manifest were produced,and
make package-verifyhad nothing coherent to report. One broken variantleft the operator with no artifacts and no written record of what broke.
That is the wrong failure mode for precisely the situation it appears in. A
library bump breaking one profile is a normal event during release prep, and
the useful outcome is "here are the three that work, here is the error for the
fourth" — the release still cannot be cut, but the blocker is legible and the
good artifacts are testable.
The change
build_prgs.shis three-valued:0all built,2partial,1nothingbuilt. It also greps the first
ld65/ca65diagnostic out of each failedvariant's log into
dist/build-info.txt, so everything downstream can statewhy a variant is missing without anyone opening a log.
build_d64.shskips absent PRGs and creates no image when none of its inputsexist, instead of erroring on the first gap.
write_manifest.shstill runs, and opens with an!! INCOMPLETE RELEASE !!block naming each missing variant, its make line and its exact error. Missing
variants are marked in the guidance list and show
FAILEDin the size table.A partial release announces itself at the top rather than being a checksum
list that merely has fewer lines than it should.
make packageruns the pipeline to completion on a partial matrix, thenprints the blocker and exits 1.
verify_release.pyleads with a BLOCKER section, reports failed variants as[n/a ]in the reproducibility check rather than as fabricated failures, andends
RELEASE INCOMPLETEwith exit 1.Problem 2 — the gate could pass having verified nothing
Chasing problem 1, LaneCerts found the sharper bug: at master,
SKIP_REBUILD=1 SKIP_VICE=1 make package-verifyprintedon a
dist/missing half its PRGs and with no disk images. Reproduced, and itdegrades further — with the build record present,
dist/emptied andSKIP_LISTENER=1as well, master's verifier prints:A gate that verified literally nothing reporting success.
Being precise about the reproducer, because the exact conditions matter.
On a genuinely clean tree, master's gate does not go green: without the
listener bundle it prints
0/1 checks passed, exit 1, becausecheck_listenerhappens to assert the bundle's presence explicitly instead of iterating. So
whether a vacuous run went green depended entirely on whether some unrelated
check happened to be written defensively:
SKIP_REBUILD SKIP_VICE0/1exit 1 (red, by luck)SKIP_REBUILD SKIP_VICE2/2VERIFIED exit 0+ SKIP_LISTENER0/0VERIFIED exit 0That inconsistency is the actual defect. 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. My first commit on this branch did not close
it: it caught variants that failed to build, and said nothing about artifacts
that were never produced — a distinction that is easy to re-collapse.
The cause is that both disk checks iterated
DIST.glob("*.d64"). No disksmeant no iterations, no records, and 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 buildrecord (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.shnow recordsbackend=per variant so this needs no second copy of the matrix.RELEASE ARTIFACTS VERIFIEDis now reserved for a run where every sectionexecuted. Any
SKIP_*yieldsPARTIAL VERIFICATION — … Not a release gate,still exit 0 so
SKIP_*stays usable for narrowing.Four cases, measured:
0/4, four "expected but absent"make packageat the broken pin6/6present verify, 2 missing,RELEASE INCOMPLETESKIP_*set4/4,PARTIAL VERIFICATIONNothing changes on a healthy, fully-run matrix: all four build, every section
runs, exit 0,
RELEASE ARTIFACTS VERIFIED.A regression test for the verdict
The verdict logic is now a pure
summarize()function withtools/test_package_verify.pypinning it — no VICE, no builds, runs inmilliseconds. 31 assertions.
Case D is the one deliberately guarded hardest, on LaneCerts's suggestion: 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
test_verdict_empty_run_not_rescued_by_skips: the skipbranch is the friendly one and exits 0, so if it were evaluated before the
empty-run branch, skipping every section on an empty
dist/would report acheerful PARTIAL — the original bug wearing a different hat. The ordering is
part of the contract and is now pinned as such.
The tests run as a preflight to
make package-verify, not just standalone— a regression test nobody runs is not a guard, and "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.
They are deliberately not in
tools/run_all_tests.py: that runner allocates aVICE instance per suite and dispatches
run_tests(transport, labels, seed), ashape a pure-logic test does not fit.
Measured against the real failure at 21f2ce5
make package(exit 1):make package-verify(exit 1):Incidental but worth recording: the two surviving REU PRGs at the v0.9.1 pin
hash identically in my worktree and in LaneCerts's, on independent checkouts —
The underlying link failure is NOT fixed here
It lives in
src/lib_contract_asserts.s, which this lane does not own. Fulldiagnosis for whoever picks it up, all measured with
od65on the builtarchives:
SHARED_PRIMITIVESSHARED_CONSUMESnistcurves-p256.a(REU)$0007nistcurves-p256-onchip.a$0005$0005$0007 -> $0005is the §8.2reu_mulbit ($0002) dropping out. This isdeliberate and documented upstream —
libs/nistcurves/CHANGELOG.md(contractv0.5.0, lib-contract #44) carries the exact table:
SHARED_PRIMITIVESSHARED_CONSUMES$0007$0007-D SHARED_REU_MUL_INIT$0005$0007-D FP_ONCHIP_MUL$0005$0005with the gating rule "profile switches (
FP_ONCHIP_MUL) drop a bit fromboth masks;
SHARED_*deferral switches drop it from the ownership maskonly." Our onchip archive measures
$0005/$0005— precisely the documentedFP_ONCHIP_MULrow. It neither owns nor consumes the REU multiply primitive,which is correct, because it has no REU mul path.
So the assert is profile-blind: it hardcodes
= APP_OWNED($0007), a valuemeasured at the v0.6.0 pin when both profiles reported
$0007. It needs aprofile-dependent expected value.
One warning for that fix: the assert's own error text says to "re-derive
the archive member drops in
tools/integration/build_nistcurves_p256.sh". Thatis the wrong remedy here and will cost someone an afternoon — nothing about the
archive surgery changed, the mask semantics were refined upstream. Worth
rewording the message while fixing the value.
🤖 Generated with Claude Code