fix(ci): make releases/latest wait for a complete asset set - #336
Conversation
Lane-opening stub for #335. Scope is `.github/workflows/**` only; PR#334 owns `src/`. Co-Authored-By: Claude <noreply@anthropic.com>
…lete A stable release is assembled by two workflows that do not wait for each other: release.yml attaches the ten raw binaries, package.yml attaches the four native install packages. Both used softprops/action-gh-release, whose make_latest defaults to true, so attaching assets also promoted the release and the promotion was won by whichever job finished first. On v0.145.0 that was package.yml at 01:47:49Z; the binaries landed at 01:52:52Z. For five minutes releases/latest was a release with no dig-node-* or dign-* binary at all, and dig-installer resolves both stems through releases/latest, so every fresh install 404'd. Nothing was red: both workflows succeeded at the job each was given. The asset guard that exists to catch this passed, because it had been written for dig-updater's feedsign and enumerated only the four package names. dig-installer was a second consumer it had never been told about. Changes: * The expected asset set moves to .github/actions/check-release-assets and covers both consumers -- fourteen names: four native packages plus dig-node-<v>-<platform> and dign-<v>-<platform> for all five platforms. * Both publishers set make_latest: false. release.yml gains a verify job (which polls until both publishers have landed, or fails) and a promote job that moves latest only after it passes. An incomplete release is therefore never latest, and the previous complete release keeps serving installs. * The guard becomes falsifiable. A self-test drives the same action over the exact four-package asset list releases/latest carried while installs were 404ing, and fails the build unless the guard rejects it. Verified locally: the old guard exits 0 on that input, the new one exits 1 naming all ten missing assets. SPEC.md 11.1 records the widened requirement and the new 11.1b promotion rule; runbooks/release.md and DEVELOPMENT_LOG.md follow. Closes #335 Co-Authored-By: Claude <noreply@anthropic.com>
…pression from a job name
The failing fixture omits both raw-binary sets, not only the dign-* one, and
the verify job name rendered as a literal ${{ }} expression on the
pull_request trigger where no tag input exists.
Co-Authored-By: Claude <noreply@anthropic.com>
…mote gotcha Promotion is now the last step of release.yml, so a published-but-not-latest stable release means the asset guard has not passed. An out-of-band re-run of package.yml demotes, because both publishers set make_latest: false. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED
Head reviewed: d8f2962 (resolved from the remote, not from the dispatch prompt). Correctness gate, fresh context, read-only — no shared checkout was mutated; every read went through git show <ref>:<path> and gh.
GitHub 422s a verdict-bearing review event from the PR author identity, so this is recorded as a comment review. The three open inline threads are what bars the merge, via required_conversation_resolution.
Verdict in one line
The diagnosis is right, the shape of the fix is right, and the regression proof is real — but the race is narrowed, not closed (a still-uploading asset satisfies the poll), and the PR adds a hand-maintained cross-repo asset table with no mechanical drift guard, in a repo that already has the idiom for one.
Findings, ranked
check-release-assets/action.yml:120— the poll reads.assets[].namewithout filteringstate == "uploaded", sopromotecan movelatestonto a release whosepackage.ymlassets are mid-upload. One jq filter. GATING.check-release-assets/action.yml:90— the 14-name expectation is a third copy of a producer contract with no mechanical guard. Additive drift fails silently, and the selftest cannot see it because it matches its own needle. Executable fix in the existinginclude_str!workflow-shape-test idiom. GATING.release.yml:5— the file header still states the release is markedlatestat publish. A superseded rule surviving in the same file. GATING, one line.
The five verification points
1. Is the race closed? At the promotion level, yes, and this was enumerated rather than sampled. Exactly two softprops/action-gh-release call sites exist repo-wide — release.yml:81 and package.yml:333 — and both now carry make_latest: "false". The only other release-writing path is nightly-release.yml:499-500, which passes --latest=false on both the create and the edit, and which is itself already guarded by nightly_release_workflow_shape.rs:211-216. Exactly one promotion site exists, release.yml:138. The workflow_call path into package.yml inherits the caller ref type and never reaches its publish job. No implicit promoter survives. At the completeness level, finding 1 leaves a residual window.
2. Can verify pass early? Not on a partial NAME set. The check is grep -qxF per expected name across all 14, so it is exact-match over the complete set, never a count and never an any-present test. Timeout is 75 minutes on the workflow_call path (5 on dispatch); on timeout it writes the missing list to the step summary, emits an error annotation and exits 1, so verify fails and promote is skipped. It can pass early only in the asset-state sense of finding 1.
3. Is the failure mode as claimed? Yes. With both publishers at make_latest: false, a release that never verifies is created but never promoted, so releases/latest keeps naming the previous complete release, and the unpromoted release stays visible for diagnosis. The runbook one-liner gh release edit vX.Y.Z --repo DIG-Network/dig-node --latest is the identical command promote runs, so it works as written. The runbook also documents the new sharp edge this creates — an out-of-band package.yml re-run now DEMOTES an already-latest release — which is the non-obvious consequence most likely to have been missed.
4. Is the expected set right for BOTH consumers? Yes, verified against producers and consumers rather than against the PR body. Producers: the two build-binaries.yml matrices emit exactly linux-x64, linux-arm64, macos-x64, macos-arm64, windows-x64.exe for both the dig-node and dign stems — an exact match for PLATFORMS, both arm64 variants included; the stable package.yml matrix builds both the amd64 and arm64 debs plus the universal .pkg and the .msi. Consumer: dig-installer/src/release.rs resolves both the dig-node and the dign stems, taking dign deliberately from the dig-node release rather than dig-app, through latest_release_api(), which is releases/latest. The strongest single piece of evidence is the live control run against a real complete release, below. Requiring arm64.deb is stricter than feedsign fails-closed, and the file justifies that explicitly against the arm64 floor tickets — correct, and worth not relaxing. The gap is the drift guard: finding 2.
5. Regression proof — re-verified independently, not taken from the PR body.
| run | tag | trigger | Verify job | evidence read from the log |
|---|---|---|---|---|
32683223388 |
v0.144.1 |
workflow_dispatch on this branch |
success in 427ms | v0.144.1 carries all 14 consumer-resolvable assets |
32683228884 |
v0.30.0 |
workflow_dispatch on this branch |
failure after polling to timeout | release v0.30.0 is missing 7 of 14 ..., naming dig-node_0.30.0_arm64.deb, dig-node-0.30.0-linux-arm64 and all five dign-0.30.0-* |
Both ran at head d8f2962, and both read a real release asset list over the network — ASSETS_OVERRIDE is empty in the passing run — so this is control-and-abuse on the live code path, not synthetic input. The failing run polled twice before failing, which also exercises the poll-then-fail path rather than a single sample. The selftest job passed on this PR as well, so the literal-list battery and the live battery agree.
The falsified defect-1 diagnosis
Correctly falsified, and it does not hide a second defect. package.yml gates its publish job on if: github.ref_type == 'tag', and the skipped occurrence was the nightly channel calling package.yml via workflow_call, where the ref type is the one belonging to the CALLER, namely branch. The nightly channel publishes those same artifacts through its own job, which uses gh release create/edit ... --latest=false and uploads the flattened package set at nightly-release.yml:439-458 — amd64 deb, macos pkg, windows msi, deliberately no arm64 deb on nightly since feedsign has one Linux entry. Nothing is dropped on either channel. Corroborated on this very PR: Attach packages to the release shows SKIPPED here for the same ref-type reason, on a run whose ref type is branch. It is a ref-type gate, not an event-name guard, and #335 should close on this PR.
Also confirmed
- Version
0.145.0to0.145.1, withCargo.lockupdated in the same commit (one line each), so--lockedcannot redden. - Scope claim holds. Nine files:
.github/actions/check-release-assets/action.yml, the three workflows,Cargo.toml,Cargo.lock,SPEC.md,runbooks/release.md,DEVELOPMENT_LOG.md. Nosrc/and nocrates/file is touched, so there is no collision with PR #334. - SPEC coherence is good. 11.1 now states that promotion is separate and gated, and enumerates the fourteen names normatively; the new 11.1b states the
make_latest: falserequirement on BOTH publishers plus the falsifiability requirement on the guard.runbooks/release.mdagrees. The only place the old rule survives is finding 3. - dig-constants check, both directions. Nothing in this diff belongs in
dig-constants, and nothing here should be consuming it: the shared values are release-asset FILE NAMES, produced by GitHub Actions YAML in this repo and consumed by Rust in two others, and a Rust constants crate cannot be read by a workflow. Finding 2 is the right centralization for this particular contract. - No UI surface, and no custody, key, spend or crypto path.
promotetakescontents: writescoped to that job alone, and the guard workflow declarescontents: read; no least-privilege regression.
Merge preconditions at this head
Do not merge on this state. Beyond the three open threads, Lint commit messages on run 32684533232 is CANCELLED in the rollup, with a later run 32684557257 SUCCESS for the same context. Assert the required set by name with .claude/scripts/check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 336 and gate on its exit code, rather than reading the rollup.
Risk note for the merging orchestrator
nightly-release.yml:80 includes github.event_name == 'schedule' in the stable job condition, so the midnight-UTC cron cuts a real stable release unattended, and the FIRST exercise of promote will have no human present. Finding 1 decides whether that first unattended cut can publish an incomplete latest, so it should land before the next midnight.
…against drift Three gate findings on #336. 1. The race was narrowed, not closed. The poll read asset NAMES with no state filter, and GitHub creates an asset row when its upload STARTS, in state `starting`. The verify job is `needs: publish`, which orders it after release.yml's own upload only -- package.yml is a separate workflow with no ordering relationship to it -- so the poll could see all fourteen names while a .msi or .pkg was still uploading and promote a release whose download is incomplete. Same observable outcome as #335, shorter window. The query now selects `.state == "uploaded"`. Deliberately not a sleep: a sleep shrinks the window without closing it and cannot be shown to fail. 2. Additive drift failed silently. The fourteen-name expectation is hand-maintained, so losing a platform fails loudly but GAINING one does not -- and the workflow self-test cannot see it, because it drives literal lists written from the same expectation and so matches its own needle. A new guard in the repo's existing include_str! idiom asserts the expectation's PLATFORMS set equals the build matrix's out_name set, that both publishers decline promotion, that exactly one site moves `latest`, and that the uploaded-state filter is present. 3. release.yml's header still said the release is "marked latest" on publish. Each new assertion was shown to fail on the defect it claims to catch, by mutating the workflows one at a time: a new build platform, a dropped make_latest, a second --latest site, and a dropped state filter each redden exactly one test. The last of those also caught a vacuity in the first version of this test -- it searched the whole YAML and was satisfied by the COMMENT that explains the filter, so it now asserts against the `gh release view` command line specifically. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
Independent re-gate of Reproduced the mutation that matters most, in my own worktree, not the lane's:
That is the distinguishing test: the earlier version of this assertion passed with the filter deleted because it scanned the whole file and matched its own documentation. The current one anchors on the line containing Also confirmed the filter is against a real field rather than a plausible-looking one — Baseline before mutation: all 5 guard tests green at Remaining mutations (matrix drift, |
|
Interim result 2/4 — finding 2 (additive matrix drift), independently reproduced. Mutation: added a single Result: FAILED, Two properties I checked beyond "an assertion exists":
Interim results 3/4 and 4/4 — the promotion guards.
Control against a false positive, which I added and the lane did not report: appended Every mutation was grep-confirmed applied before the run (a |
PASS — independent scoped re-gateHead reviewed: All three findings from the prior round are genuinely fixed, verified by mutation in my own worktree rather than by reading the assertions. The lane resolved its own gate's threads, so every claim below was re-measured independently; the four mutations the lane reported are reproduced, plus one control it did not run. Finding 1 — the state race is CLOSED, not narrowed
Finding 2 — additive drift now fails loudly
Divergence test I ran: added Promotion guards, also reproduced: deleting Control the lane did not report: appending a legitimate Every mutation was grep-confirmed applied before its run, and the tree restored clean after each. Finding 3 — header matches behaviour
Delta scope, version, checks
One NON-GATING observation (I am resolving this myself; it does not bar merge)SPEC §11.1b defines the promotion contract and its falsifiability clause, but "verified complete" is not defined to include upload state — it requires only that the self-test fail an asset list carrying just the native packages. The Suggested one-sentence addition while the PR is still draft (cheap, and Deliberately non-gating: it is an omission rather than a contradiction, the operational risk of merging without it is nil, and the tonight midnight-UTC cron cuts a real stable release unattended — holding a correct fix for a doc sentence would leave that cut unguarded, which is strictly worse. |
All required checks green, asserted by name via
check-merge-preconditions.sh(exit 0); zero unresolved threads.Risk note for the merging orchestrator: this changes the release path itself, and dig-node's midnight cron cuts a real stable release unattended, so the first cut after merge exercises the new
promotejob for real. It fails safe — ifpromotedoes not run, the release is published but notlatest, and the previous complete release keeps serving installs.runbooks/release.mdrecords the one-line manual re-promote.Closes #335.
What actually happened
releases/latestwas not missing its assets permanently. It was missing them for 5m03s, and the ticket measured inside that window.01:26:07Zv0.145.0pushed; bothrelease.ymlandpackage.ymlruns created01:47:47Zpackage.ymlattaches the 4 native packages01:47:49Zpackage.ymlpromotes the release tolatest— with no binaries on it01:52:52Zrelease.ymlattaches the 10 rawdig-node-*/dign-*binaries01:53:00Zrelease.ymlcompletesBetween
01:47:49Zand01:52:52Z,releases/latestwas a release carrying four.deb/.pkg/.msifiles and zero binaries. dig-installer resolves both thedig-nodeanddignstems throughreleases/latest(dig-installer/src/release.rs:187, stems at:59/:78), so every fresh install 404'd for that window. Nothing was red, because both workflows genuinely succeeded at the job each was given.Cause: a stable release is assembled by two workflows that do not wait for each other, and both used
softprops/action-gh-release, whosemake_latestdefaults totrue. Attaching assets was promoting the release, and the promotion was won by whichever job finished first.The ticket's defect 1 is falsified
The skipped "Nightly — native packages / Attach packages to the release" job in run
32679750245is correct and by design, not the cause. That is the nightly channel'sworkflow_callintopackage.yml, whosepublishjob isif: github.ref_type == 'tag'— and aworkflow_callinherits the caller's ref type, which isbranch. The nightly channel publishes those artifacts through its own job. The stable path's attach ran normally in run32679766273(event=push,success).Defect 2 is real and is what this PR is mostly about
"Stable — verify release assets" passed because its expected set was only the four native package names. It was written for dig-updater's feedsign; dig-installer is a second consumer nobody had told it about. It was satisfied at
01:47:47Z— two seconds before the incomplete release becamelatest.The fix
.github/actions/check-release-assetsand covers both consumers: 14 names — 4 native packages, plusdig-node-<v>-<platform>anddign-<v>-<platform>for all five platforms.release.ymlandpackage.ymlboth setmake_latest: false.release.ymlgains averifyjob (which polls until both publishers have landed, or fails) and apromotejob that moveslatestonly after it passes. An incomplete release is therefore neverlatest; the previous complete release keeps serving installs, which is the correct failure mode.selftestjob drives the same action over the exact four-package asset listreleases/latestcarried while installs were 404ing, and fails the build unless the guard rejects it.Proof the fixed guard goes red
Both scripts extracted from the YAML and executed, so this is the real code path, not a paraphrase:
The failing case names exactly the ten assets #335 measured as 404.
And live, against real releases
The literal-list self-test could in principle drift from the live path, so the fixed guard was also dispatched against two real tags on this branch, reading their actual asset lists through
gh release view:32683223388v0.144.1(complete, 14 assets)32683228884v0.30.0(nodign-*at all, no arm64)The v0.30.0 failure names the seven real gaps:
dig-node_0.30.0_arm64.deb,dig-node-0.30.0-linux-arm64, and all fivedign-0.30.0-*. Control and abuse, on the same code path the release run uses.The self-test job also passes on the runner (7s) on this PR, and
package.yml'sAttach packages to the releaseshowsskippinghere — independently confirming the ref-type gate is what skips it, not an event-name guard.v0.145.0 today — fetched, not inferred
All 14 assets present. Each fetched through GitHub's 302 redirect with
Content-Lengthmatched against the API-declared size:dig-node-0.145.0-linux-x64dign-0.145.0-linux-x64dig-node-0.145.0-windows-x64.exedign-0.145.0-macos-arm64dig-node_0.145.0_arm64.debNo repair action was needed — the assets self-completed at
01:52:52Z.Blast radius
gitnexus
impactis not applicable: this diff contains no source symbols. Radius established by grep instead —verify-release-assets.ymlis referenced bynightly-release.yml:322(unchanged, and it now inherits the widened check) and byrunbooks/release.md;make_latestappears only in the two publishers changed here and inSPEC.md. All four call sites updated. Nosrc/file is touched.Scope
.github/workflows/**+.github/actions/**, plusSPEC.md/runbooks/release.md/DEVELOPMENT_LOG.mdcoherence. Nosrc/change — PR #334 owns that. Version: smallest patch bump,0.145.0→0.145.1, required by the version-increment gate; expect to rebase behind #334.Round 2 — all three gate findings addressed (
d717c25,6cd9090)Finding 1 — the race was narrowed, not closed. The poll read asset names; GitHub creates the asset row when an upload starts (
state: "starting"), andverifyisneeds: publish, which orders it against release.yml's own upload only —package.ymlhas no ordering relationship to it. The query now selects.state == "uploaded". No sleep: it would shrink the window without closing it and could not be shown to fail. Verified against the live API (gh release viewdoes exposestate; the filtered query returns 14 onv0.144.1), then re-dispatched live — run32686543775@v0.144.1success, run32686550399@v0.30.0failure naming its seven real gaps.Finding 2 — additive drift. New guard
crates/dig-node-service/tests/release_asset_matrix_guard.rs, in the existinginclude_str!idiom. Asserts thePLATFORMSset equals the build matrix'sout_name:set (equality both ways — a superset wedges every release, a subset is the silent hole), that everyaction-gh-releasestep declines promotion (count-matched), that exactly one--latestsite exists and is inrelease.yml(--latest=falsecounted separately as a demotion), and that the state filter is present. Plus a non-triviality control, since two empty scrapes compare equal.Each assertion was shown to fail on the defect it claims to catch, by mutating one workflow at a time — a new build platform, a deleted
make_latest, a second--latestsite, and a dropped state filter each redden exactly one test.That last mutation caught a vacuity in the guard itself: the first version searched the whole YAML and passed with the filter deleted, because the comment explaining the filter contains the same text. It now asserts against the
gh release viewcommand line specifically.Finding 3 — one line.
release.yml's header no longer claims the release is markedlateston publish.All five required checks green, asserted by name (
Lint commit messageshad a CANCELLED run in the rollup; the gate script reads branch protection and the later SUCCESS). Three review threads resolved. Held DRAFT pending re-gate.