fix(release): assert the installers actually staged before publishing - #150
Conversation
`find ... -exec cp` exits 0 when it matches NOTHING, even under `set -euo
pipefail`. The two steps that would otherwise notice an empty stage —
`attest-build-provenance` over out/*.exe|*.pkg|*.sh, and `gh release create` —
are BOTH gated on `publish == 'true'`. A `workflow_dispatch` dry run therefore
staged only the two lock files, skipped the gated steps and reported GREEN: the
rehearsal that exists to build confidence before a signed release could not
detect its own most important failure.
Add an unconditional completeness gate to the staging step. `EXPECTED_LEGS`
declares one `<platform>:<ext>` entry per `build` matrix leg and is the single
source of truth for what a complete release contains. The gate requires a
per-leg `SHA256SUMS-<platform>.txt` arrival receipt, compares per-extension
installer counts by exact equality, and rejects an empty bill-of-materials.
Per-extension rather than a total of 4: the two macOS legs are distinguished
only by the arch constructor embeds in the .pkg name, and `merge-multiple: true`
silently overwrites same-named files — so 2 .exe + 1 .pkg + 1 .sh still totals 4
while the Intel-Mac installer is missing. Exact equality also catches
over-collection. The step now logs the downloaded `dist/` tree first, because
the artifact layout (dist/dist/<installer>, exactly at the `-maxdepth 2` limit
with zero headroom) is the thing that silently drifts.
`EXPECTED_LEGS` is parsed with `read -ra`, not unquoted word splitting: with
`shopt -s nullglob` enabled an unquoted leg containing a glob metacharacter is
DELETED rather than left literal, which would shrink the expected set and make
the gate fail OPEN. Failure terminates with a bare `false`, never `exit`, which
reports "exit code 1" under the `bash -el {0}` login shell used elsewhere here.
Five contract tests in tests/test_marker_contract.py keep the gate honest: they
bind `EXPECTED_LEGS`' platforms to the build matrix and its extensions to
`installer_type:` in packaging/construct.yaml, forbid `if:`/`continue-on-error`
on the gate (a dry run is precisely the run that needs it), and assert the
comparison SHAPE rather than a token that also appears in the error strings.
Verified by execution against six simulated artifact trees: happy path passes;
depth-3 layout drift, a missing osx-64 leg, a wrong 4-installer mix, a zero-byte
lock and an empty dist/ all fail loudly.
Refs: PRD §9 M9, §12.7; ADR-0050.
📝 WalkthroughWalkthroughChangesRelease completeness validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseJob
participant DistArtifacts
participant OutputDirectory
participant ContractTests
ReleaseJob->>DistArtifacts: Inspect downloaded artifact tree
ReleaseJob->>OutputDirectory: Stage installers, checksums, and frozen locks
ReleaseJob->>OutputDirectory: Validate expected legs and artifact completeness
OutputDirectory-->>ReleaseJob: Report missing or mismatched artifacts
ContractTests->>ReleaseJob: Verify workflow structure and gate contracts
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review The adaptive rate-limit window quoted earlier (47 min) has elapsed. Requesting the substantive review now — the earlier green |
|
You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 59 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Summary
find ... -exec cpexits 0 when it matches nothing, even underset -euo pipefail. The two steps that would otherwise notice an empty stage —attest-build-provenanceoverout/*.exe|*.pkg|*.sh, andgh release create— are both gated onpublish == 'true'. So aworkflow_dispatchdry run stages only the two lock files, skips the gated steps, and reports green: the rehearsal that exists to build confidence before a signed release could not detect its own most important failure. This adds an unconditional completeness gate to the staging step so it fails loudly on an incomplete release — on a dry run too. Relevant now, since the pipeline has never run end-to-end and its first exercise will be a dry run before cuttingv1.0.0.Linked tracking
Type of change
feat— new capabilityfix— bug fixdocs/chore/ci/build/refactor/test/perf!/BREAKING CHANGE:— a deliberate schema-version bump (the only sanctioned breaking change)Self-review checklist (PRD §12.4)
.tetherHDF5 change; CI-only (release.yml+ a test).schema-guardgreen.conda-lock-verifygreen.tests/test_marker_contract.py(stdlib-only, OS-agnostic, required 3-OS matrix); CodeQL clean. No GUI surface..tether; this hardens the release-artifact staging path.EXPECTED_LEGSis derived from the build matrix +construct.yaml, and the contract tests bind it to both.GPL-3.0-or-later); no scientific claims;reuse lintunaffected.What
An unconditional completeness gate in the staging step.
EXPECTED_LEGSdeclares one<platform>:<ext>entry perbuildmatrix leg and is the single source of truth for what a complete release contains. The gate:SHA256SUMS-<platform>.txtarrival receipt — the only per-platform evidence available, since installer filenames use constructor's own naming (Tether-1.0.0-MacOSX-arm64.pkg) which doesn't map back to a matrix key;.pkgname, andmerge-multiple: truesilently overwrites same-named files, so2 .exe + 1 .pkg + 1 .shstill totals 4 while the Intel-Mac installer is missing;dist/tree first (the artifact layout is the thing that silently drifts).EXPECTED_LEGSis parsed withread -ra, not unquoted word splitting: withshopt -s nullglobenabled an unquoted leg containing a glob metacharacter is deleted rather than left literal, which would shrink the expected set and make the gate fail open. Failure terminates with a barefalse, neverexit(which reports "exit code 1" under thebash -el {0}login shell used elsewhere in the workflow).Testing
pytest tests/test_marker_contract.py→ 21 passed (16 existing + 5 new);ruff check+ruff format --checkclean at 0.15.19; all touched YAML parses.The
run:script was extracted from the YAML and executed against six simulated artifact trees:osx-64leg.exe+ 1.pkg+ 1.sh(total = 4)conda-lock.ymldist/Five mutation checks on the contract tests (stale
EXPECTED_LEGS,if:-gated, neutered comparison,false→exit 1, unquoted loop) each turn exactly one test red.What this does NOT cover
.exepasses every count — and the unconditional combined-SHA256 step then re-hashes it, laundering the corruption. The staged per-platformSHA256SUMS-<platform>.txtfiles are never verified withsha256sum -c. Worth a follow-up.dist/locks/are downloaded and dropped by the find filter —docs/release.mdpromises "the frozen conda locks", which the two source locks satisfy (a documented smell, not a contradiction).-maxdepth 2layout claim comes from upstream docs, not an observed run. The gate asserts the outcome, so it is correct at any depth; only the comment could be wrong, and the first dry run's::group::output confirms or corrects it for free.