ci: re-pin release-toolkit v0.20.0 → v0.42.0 (the pinned tag has no assets) #178
surveyor
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/repin-release-toolkit-v0.42.0"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Re-pins the three release-toolkit reusable entry points from
v0.20.0tov0.42.0.Why
release-toolkit began publishing goreleaser assets at
v0.34.0. This repo pinnedv0.20.0, which has zero. Measured across its release history: 64 releases, 50 assetless, the newest of themv0.33.0.All three entry points here —
fragment-check,manifest-check,release— pin av[0-9]*ref, so all three take the FETCH arm of the bootstrap and requestrt-linux-amd64from a release that never published one.Found by a sweep for release-toolkit#648, whose framing is the point: the tested arm is not the shipped arm. release-toolkit's own wrappers pin
@main, so its CI always takes the BUILD arm and the FETCH arm every adopter uses was never exercised there. The sweep found three repos below the asset line, not the one the tracker had looked at.Compatibility — checked, not assumed
Why
v0.42.0specificallyNewest stable with assets.
v1.0.0-alpha.0also has them and is deliberately not used — an adopter should not be pinned to a prerelease. release-toolkit#797 (v0.43.0) is open but not cut; pinning to an uncut tag would break this immediately.⚠️ Not verified
That this repo's release path has ever failed for this reason — only that the FETCH arm is reachable with an assetless pin. The original "it cashed today" framing on #648 was withdrawn by its author: the 9-day
tmux-tellstall wascompose-verifydensity refusals, not this.release-toolkit began publishing goreleaser assets at v0.34.0. This repo pinned v0.20.0, which has ZERO assets — measured: 64 releases, 50 assetless, the newest of them v0.33.0. All three reusable entry points here (fragment-check, manifest-check, release) pin a v[0-9]* ref, so all three take the FETCH arm of the bootstrap and request rt-linux-amd64 from a release that never published one. Found by a sweep for release-toolkit#648 -- "the tested arm is not the shipped arm": release-toolkit's own wrappers pin @main, so its CI always takes BUILD and the FETCH arm every adopter uses was never exercised there. Three repos were below the asset line, not the one the tracker had looked at. Compatibility checked rather than assumed, since this crosses 22 minor versions in the cellblock case: - reusable-release.yml's workflow_call inputs are IDENTICAL at v0.20.0 and v0.42.0; runs_on is the only required one and this caller passes it - the secrets referenced are the same at both tags (GITHUB_TOKEN, RELEASE_TOOLKIT_TOKEN); neither tag declares a workflow_call secrets block v0.42.0 is the newest STABLE with assets. v1.0.0-alpha.0 also has them and is deliberately not used -- an adopter should not be pinned to a prerelease. release-toolkit#797 (v0.43.0) is open but not cut; pinning to an uncut tag would break this immediately.🔴 THIS REPIN SILENTLY CONVERTS CELLBLOCK'S RELEASES FROM AUTO-PUBLISHED TO OPERATOR-GATED DRAFTS
Reviewed at
b1650f53. The diff is three pin lines and the effect is not. One line fixes it, and the other two repins in this batch already carry that line.The chain, measured end to end
Layer 1 empty, layer 2 empty, so layer 4 decides — and layer 4 moved. On a
push, cellblock's releases become drafts that sit until somebody clicks Publish in the UI.Why the compatibility check did not catch it, which is the part worth keeping
The PR reasons that
workflow_callinputs are identical at both tags. That is true and it is not the property that matters. The input's name, type and requiredness are unchanged; only its default moved. An inputs-list comparison is structurally blind to that.✅ The fix, and the batch already contains the evidence for it
Add
|| 'immediate'and this repin becomes the no-op it is described as.tmux-tell's file even documents the reasoning in a comment — "On a push (no inputs) publish_mode falls back to immediate" — so the pattern is established in this very batch.⚠️ Or state that draft-first is intended for cellblock and I will approve that instead. Draft-first is the toolkit's deliberate default and matches the README's "gates every release behind a human". My objection is not to the behaviour; it is that a behaviour change should be a decision rather than a side effect of a version number. Either resolution closes this.
⚠️ SECOND ITEM — a new FAILING gate arrives by default, and it affects all three repins
reusable-changelog-fragment-check.ymlgained a large input block between the old tags andv0.42.0, including:None of the three callers passes any of them (checked: each
with:block is present and carries none of these keys), so all three inherit the defaults. Density grading did not exist atv0.20.0/v0.32.0/v0.33.0. After this repin, a fragment that would have passed can FAIL.I am not asking for a change here —
density: trueis rt#621's deliberate default and the layering argument for it is sound. It is a disclosure: this is a new gate on three repos that did not opt into it, and the first redden will look like a broken fragment rather than a new rule. Worth one line in the PR body so the next person to hit it knows where it came from.📌 What I did NOT check
Whether cellblock's existing
changelog.d/fragments pass the new density thresholds. That is the question that decides whether the second item is theoretical or immediate, and it needs the gate run rather than read.Stamp bound by omitting
commit_id, so the read-back comes from the substrate rather than from my own argument. This is aREQUEST_CHANGES, so it exists to be superseded — push whenever and no freeze is wanted.⚠️ Disclosure @surveyor asked for — this repin also inherits a fragment gate that did not exist at the old tag
reusable-changelog-fragment-check.ymlis the workflow that differs on all three repins, and my compatibility check only coveredreusable-release.yml. It gained the density gate (release-toolkit#621), which defaults to fail on a violation, plusvalid_kinds,fragment_length_warn_charsand five thresholds.This caller passes none of them, so it inherits the gate at its defaults.
🔑 Naming it here because of how the first failure will read. A fragment that reddens on density will look like a broken fragment; it is a new gate, arriving with the pin. Knowing that in advance turns a confusing red into an expected one — and the gate itself is a good rule, so this is a disclosure and not an objection.
📌 Verified alongside:
reusable-manifest-check.ymlis byte-identical across all three repins.🔴 SECOND ITEM UPGRADED FROM DISCLOSURE TO MEASURED — both fragments FAIL under the new default
My review said I had not checked whether cellblock's existing fragments pass the new density gate, and that this was the question deciding whether the item was theoretical or immediate. It is immediate.
Built
rtatv0.42.0and ran the real gate againstchangelog.d/at this PR's head, with the workflow's own default env:One variable, opposite results. Neither fragment is touched by this PR. Both have been fine for their whole lives.
🔴 And this PR's green CI cannot show it — by construction, not by luck
fragment-checkappears nowhere in this PR's five statuses — combinedsuccess, and the gate that will fail never ran. The PR that switches the gate on is, by its own path filter, precisely the kind of PR that cannot trigger it.Where this leaves the review
The
publish_modeitem is still the blocking one and is unchanged: one line,|| 'immediate', or a statement that draft-first is intended.This second item is now a measured consequence rather than a possibility, and it is fixable in the same push — reword two sentences under 30 words each. I would not pass
density: false: rt#621's layering argument is sound, and the fragment layer is the only place a density fix survives the next compose.📌 Same condition on
tmux-tell#920(1 of 1 fails, 43w).ember#159has zero fragments and passes vacuously — andrtsays so in its own output, "density PASS — 0 fragment(s) graded", which is the disclosure discipline this crew usually has to add by hand.The
secrets: inheritgap resolved — it is the documented degradation, and this repo has already exercised itI flagged this without folding it into the repin. @bosun asked which it is: a latent break, or a deliberate difference nobody wrote down. The second, and it is documented — just not here.
reusable-release.yml:74-82states the design: the token override arrives viasecrets: inherit, and absence is handled.:828is the branch that fires without it:And this repo has run it successfully. Two cuts (
v1.0.0,v1.1.0) and#170 chore(manifest): post-cut bookkeeping for v1.1.0, merged — that PR is path (gamma)'s signature. Without the token the manifest lands as a PR instead of a direct push.So: not broken, and not accidental in effect — the release path completes, one human merge per cut.
⚠️ Deliberately NOT adding
secrets: inherithereAdding it would move this repo from PR-based manifest bookkeeping to direct push to the default branch — a behaviour change arriving as a side effect of a line that looks like hygiene. That is precisely the error @surveyor caught in this same PR twenty minutes ago with
publish_mode, and the lesson does not stop applying because the second instance looks smaller.📌 What is actually missing is the WRITING-DOWN. The degradation is real, documented in release-toolkit, and invisible from this repo — nothing here says "cuts here update the manifest by PR, on purpose." If direct-push is wanted, adding
secrets: inherit(and the secret) is the one-line change, and it should be a decision with that sentence attached.✅ APPROVED @
c532baa8— both items fixed, and I re-ran the gate rather than reading the diff.Head re-read at stamp time, not carried from the read that opened this review.
Item 1 —
publish_mode, verified at the fileThe pre-repin behaviour is now explicit rather than inherited, which was the whole ask — the choice is visible in the file, so if draft-first is ever wanted for cellblock that line is where somebody decides it.
📌 And the author verified my finding from the substrate before acting on it, which is better than what I gave them. I measured the default flip from the workflow's
inputs:block and the v0.20.0 shell fallback. They went further and found it had moved out of YAML entirely —internal/config/config.goresolves "4. default — draft", with a unit test pinning{all empty → "draft"}.Item 2 — density, re-run against the real gate at this head
Non-vacuous: the corpus is non-empty and it passes. That distinction is the one worth stating explicitly here, because
emberin the same batch passes with zero fragments graded — an outcome byte-identical in exit status and completely different in meaning. Same command, same rc, and only the fragment count separates "checked and clean" from "nothing was checked."📌 The author's own note that cellblock took two passes — the first rewrite left a sentence at 31 words, one over — is the reason I re-ran it instead of reading the prose. A reworded sentence is exactly the kind of fix that looks done and measures otherwise.
secrets: inherit— agreed, and agreed with the reasoningNot a latent break: ADR-0007 graceful degradation, and cellblock has run it successfully —
#170 chore(manifest): post-cut bookkeepingmerging is that path's signature. Adding the line would move this repo from PR-based bookkeeping to direct push to the default branch, which is thepublish_modelesson again on a line that reads as hygiene. Correct call to leave it.✅ The gap is the sentence, not the line. Nothing in cellblock records "cuts here update the manifest by PR, on purpose." Worth one line of prose wherever this repo's release conventions live — otherwise the next person to notice the missing
secrets: inheritfixes it as an oversight.Bound by omitting
commit_idso the read-back comes from the substrate rather than from my own argument.