[fix](release) Upload only regular files so nested supplemental assets attach

[dchaudhari7177]

Closes #550.

Root cause

release-supplemental is packed from two source directories:

path: |
  release-assets/*
  packages/*.sigstore.json

With two roots there is no common prefix to strip, so the artifact keeps its packages/ and release-assets/ prefixes. release-distributions, by contrast, is packed from packages/*.tar.gz + packages/*.whl — one common prefix, so it flattens.

Both download into release-files/, which is why the tree is mixed: loose wheels at the top, plus two directories. release-files/* then handed gh release upload those directories, and it refuses them — but only after uploading the assets that sorted ahead alphabetically, which is why three wheels landed before the failure.

Fix

Enumerate regular files recursively (find release-files -type f) and pass those explicitly. Plus three guards the previous one-liner had no room for:

1. Empty release-files/ fails loudly instead of "succeeding" with nothing attached.
2. Colliding basenames are refused. Release assets are a flat namespace keyed by basename; the tree being flattened is not. Two same-named files in different directories (say a SHA256SUMS in both release-assets/ and packages/) would overwrite each other under --clobber and publish a release quietly missing one. This isn't in the current layout, but flattening a tree into a flat namespace is exactly where it would appear later.
3. Post-upload verification as its own step, before Publish the complete draft release. Steps fail fast, so the publish can no longer promote an incomplete draft.

The verification is a set comparison, not a hardcoded count — the issue mentions six distributions and nine supplemental assets, but pinning 15 would need editing every time the build matrix changes. Comparing local filenames against gh release view --json assets catches a missing asset regardless of how many there should be.

Verification

I can't run the release workflow, so I reproduced the artifact layout locally and exercised the logic directly.

The bug, reproduced — same shape as the issue:

=== OLD behaviour: release-files/* ===
  DIRECTORY -> gh fails: release-files/packages
  DIRECTORY -> gh fails: release-files/release-assets
  file: release-files/vane-0.1.0-cp312-cp312-linux.whl
  ...
=== NEW behaviour: find -type f ===
  count: 7   (all regular files, no directories)

All three guards fire, and the happy path doesn't false-positive:

=== duplicate-basename guard ===  CAUGHT collision: SHA256SUMS
=== empty-dir guard ===           CAUGHT empty
=== missing-asset verify ===      CAUGHT missing: b
=== verify passes when complete === OK, no missing

release.yml parses as YAML, and the step order is Attach → Verify → Publish.

find -printf is GNU find, which ubuntu-24.04 has.

Note on the alternative fix

This could instead be fixed at the upload end, by flattening release-supplemental when the artifact is built (two upload-artifact steps, or staging both sets into one directory first). I went with the download end because it is the smaller change and keeps the artifact contents traceable to their source directories. Happy to switch if you'd rather the artifact were flat.

[kaka11chen]

The recursive regular-file upload fixes the root cause, and the empty-set/basename-collision guards are useful. I am requesting changes because the release verification still fails open in several cases and does not enforce the asset-integrity requirements from #550. Please address the inline comments and add a committed regression test covering the nested artifact layout, empty/colliding inputs, extra remote assets, and size/digest mismatches.

[kaka11chen]

[P2] Please propagate failures from asset discovery. mapfile returns its own status and does not propagate the status of this process substitution, so find or sort can emit a partial list and fail while the step continues with a non-empty assets array. Build the sorted list with a directly checked command (preferably NUL-delimited into a temporary file), then load it with mapfile.

[kaka11chen]

[P2] Please do not evaluate this assignment as part of the if condition. Commands in an if condition are exempt from set -e; if xargs, basename, sort, or uniq fails, this condition simply evaluates false and the upload continues. Assign duplicates in a separate statement so set -euo pipefail can stop the step, then test whether it is non-empty. The missing="$(comm ...)" check below has the same fail-open pattern.

[kaka11chen]

[P1] Please verify the exact asset inventory and contents before publishing. comm -23 only proves that local names are a subset of remote names: it still passes with stale extra remote assets, same-named assets having the wrong size/content, or an incomplete local artifact set. This also falls short of #550's acceptance criterion to verify the expected 15 unique assets by name, size, and SHA-256. Generate a normalized local manifest of basename, size, and sha256:<digest>, obtain the corresponding name, size, and digest fields from gh release view --json assets, validate the expected inventory, and compare the two manifests for exact equality.

[dchaudhari7177]

Thanks — all three fail-open patterns were real, and the verification is now inventory- and content-based per #550. Pushed as 3aaeeec.

I reproduced each pattern before changing anything, under set -euo pipefail:

--- 1. mapfile hides find's failure ---
  survived; assets=0 (set -e did NOT fire)
--- 2. assignment inside if is exempt from set -e ---
  condition read false, step continues -- failure swallowed
--- 3. comm -23 passes with stale extra remote assets ---
  missing='' -> verification PASSES despite the extra remote asset

[P2] Asset discovery. Collection moved into a script that exits non-zero on any problem, so set -e fails the step. The workflow reads its output with mapfile -d '' -t assets < upload-list — NUL-delimited, so a name containing whitespace survives, and mapfile's own status is now the only thing its exit code has to mean.

[P2] The if-assignment guards. Both are gone. The collision guard is in Python and raises; the missing="$(comm ...)" check is replaced entirely (below). Nothing is assigned inside an if condition anymore.

[P1] Exact inventory and contents. scripts/check_release_assets.py builds a local manifest of (basename, size, sha256), asserts the expected inventory — one sdist, five wheels, a Sigstore bundle paired to each of those six distributions, SHA256SUMS, the SBOM and the provenance bundle, 15 unique assets — and requires exact equality with the name/size/digest triples from gh release view --json assets. That closes the three holes you named: extra remote assets are now an error rather than an ignored superset, a truncated upload fails on size, and a substituted asset fails on digest.

Two judgement calls worth surfacing:

• Inventory is checked by shape, not pinned filenames (1 *.tar.gz, 5 *.whl, <dist>.sigstore.json for each) so a version bump doesn't have to touch the script. It's still strict enough that a missing wheel or an unsigned distribution fails. Say the word if you'd rather it pin exact names against pyproject.toml the way check_release_artifacts.py does.
• An asset gh reports with an empty digest is refused, not skipped. Skipping it would be the same fail-open in a new place. If assets can legitimately report no digest while still processing, this needs a retry instead — I don't have a release to observe that on, so I chose the strict reading.

Regression test: 24 cases in tests/fast/test_release_asset_manifest.py, built on the real post-download layout from #550 (flat release-distributions, nested packages/ and release-assets/ from release-supplemental) — covering nested collection, directories excluded, empty tree, colliding basenames, each missing asset class, an unexpected extra local asset, stale extra remote assets, size mismatch, digest mismatch, and both CLI exit codes. All 24 pass; ruff check and ruff format --check clean.

One gap to flag honestly: I can't run the full tests/fast suite locally — tests/conftest.py imports vane, which needs the native extension built, so I ran this file with --noconftest. It has no vane import and needs none, but that's CI's check to confirm, not something I verified.

[dchaudhari7177]

All three inline findings are addressed in 3aaeeec. The shape changed: rather than patching the shell, the collection, inventory assertion and verification now live in scripts/check_release_assets.py with a committed test file — which is what makes the fail-open cases testable at all.

[P2] mapfile swallowing failures. Gone. collect writes a NUL-delimited list to a file and exits non-zero on any problem, so set -e fails the step before the upload. mapfile now only reads a file that a checked command already produced; its own status is the only one that matters.

[P2] Assignments inside if. Also gone — there is no if around a command substitution left in either step. The duplicate-basename and inventory checks are exit codes from the script, not shell conditionals, so they cannot evaluate to false and continue.

[P1] Exact inventory and contents. comm -23 is replaced by a two-sided manifest comparison on (name, size, sha256). Both sides must match as sets, so all four of the holes you named now fail: stale extra remote assets, same-named assets with wrong size, same-named assets with wrong content, and an incomplete local set. The expected inventory from #550 is asserted independently — 1 sdist, 5 wheels, a Sigstore bundle per distribution, plus SHA256SUMS, the SBOM and the provenance bundle, for 15 unique assets.

I could not run tests/fast/test_release_asset_manifest.py on this machine (the conftest imports vane._native._sqltypes, which needs the compiled extension), so I exercised the script directly against a synthetic 15-asset tree instead:

collect                  rc=0   collected 15 asset(s)
verify, exact match      rc=0   verified 15 asset(s) on the release by name, size and sha256
verify, wrong size       rc=1   size mismatch for vane_ai-1.0.0.tar.gz: local 31, release 999
verify, wrong digest     rc=1   digest mismatch for vane_ai-1.0.0.tar.gz: local sha256:17c78dfb...
verify, stale extra      rc=1   unexpected asset on the release: stale.txt
verify, missing asset    rc=1   missing from the release: vane-ai-build-provenance.sigstore.json

The committed tests cover the same five plus the nested layout, empty input, and basename collisions across packages/ and release-assets/. Every failure names the offending asset rather than reporting a count, since a release step that fails at 3am should say which file.

One judgement call worth flagging: EXPECTED_WHEELS = 5 is a hard-coded constant. That is deliberate — a missing wheel is exactly the failure #550 is about, and deriving the number from what's present would make the check unable to see it — but it does mean adding a Python version requires touching this script. If you'd rather derive it from the build matrix, say so and I'll wire it up.

[dchaudhari7177]

Closing this as superseded by #818, which landed the same fix for #550 (nested asset collection with name, size and SHA-256 verification before publish) and also covers resuming partial uploads. Thanks for the reviews here.