frankenbit/release-toolkit

Fork 0

fix(reusable-release): #637 pre-check the manifest against the branch, verify it landed #640

Merged

bosun merged 1 commit from i/637-manifest-writeback-precheck-source into main

2026-08-01 00:57:57 +02:00

engineer commented

2026-08-01 00:53:41 +02:00

Owner

Refs #637. Shape 1 per the ruling — with one correction to the literal form.

The defect

:479  PRECHECK_DECISION=$(rt manifest-precheck "$MANIFEST_PATH_INPUT" "$HEAD_SHA")
                                                ^^^^^^^^^^^^^^^^^^^ the WORKING TREE

rt release writes the manifest itself (internal/release/interface.go:64, "keyed on
the merge SHA"). scripts/draft-release.sh never did — zero manifest mentions — so
before 6547d9c (bash→rt, inside the bisect window) the pre-check always saw the
previous cut's SHA and proceeded.

After it, the file already records HEAD_SHA by the time the guard runs. The guard reads
its own run's write, concludes "a prior run already landed this", and skips the
commit and both pushes. The change dies with the workspace.

⚠️ `git show HEAD:` is the intuitive fix and is silently wrong

The bookkeeping commit is a child of the cut commit — verified: a1bf251's parent is
f38267e, and the manifest at the cut commit reads the previous release:

manifest AT the cut commit (f38267e)   last_released_version=0.32.0
manifest AFTER bookkeeping (a1bf251)   last_released_version=0.33.0

So HEAD: can never contain this cut's bookkeeping. The pre-check would always
proceed, disabling #128/#134 idempotence entirely — with no symptom, on operator re-run.

The guard's question is "did the commit LAND", which is a fact about the branch. It
now reads the fetched remote tip (FETCH_HEAD), which is immune both to the workspace
write and to any future step reordering.

Unreadable (bootstrap, or fetch failure) ⇒ proceed. A pre-check that cannot read must
not silently skip; the post-condition is what refuses.

Post-condition

After the push, asserts the bookkeeping commit is on the default branch — the
destination, not the step. This failure was green twice, so an assertion that the step
ran would have been green too. It refuses: a warning could not have stopped either
bad cut.

Neither component is wrong alone

rt manifest-precheck is correct. On the real cut-time inputs both implementations
agree:

manifest eff2d9d1 vs HEAD_SHA ff2f352b   →   bash: proceed    go: proceed

It was simply handed a manifest that already said ff2f352b.

A differential oracle cannot see this class. It compares the ported unit against its
bash twin and says nothing about what the caller now hands it. That blind spot applies
to the other six migrated callsites, not just this one — noted on the tracker.

Mutation matrix — each arm fails for its own reason and only its own

precheck → working tree      arm 21 RED    22 green   23 green
post-condition → warning     arm 21 green  22 green   23 RED
precheck → HEAD:             arm 21 green  22 RED     23 green

File restored byte-identically (sha256 compared) after each mutation.

Gate

gofmt / go vet / go test -count=1 ./... / shellcheck --severity=warning /
bats tests/ (845) / register-check / fragment-check — all green. Workflow YAML
parses. Fragment density 7/8/9 pass.

What this PR does NOT do

Does not change rt release. It still writes the manifest; that write is now
simply not mistaken for evidence of a landed commit. Changing it would alter rt
behaviour for three external consumers to restore a bash-era contract.
Does not move the pre-check earlier. That re-couples the guard to step ordering,
which is what broke here — it would make the bad state carefully avoided rather than
unrepresentable.
Does not touch the preflight-push-whitelist warning. "cannot resolve push
identity (token owner); WARN + proceed" fired on this same run, on the exact
credential path the skipped push would have used. Not implicated — the push never ran —
but it becomes load-bearing once the push executes again. Recorded on #637 as
adjacent, deliberately not folded in.

Refs #637. Shape 1 per the ruling — with one correction to the literal form. ## The defect ``` :479 PRECHECK_DECISION=$(rt manifest-precheck "$MANIFEST_PATH_INPUT" "$HEAD_SHA") ^^^^^^^^^^^^^^^^^^^ the WORKING TREE ``` `rt release` writes the manifest itself (`internal/release/interface.go:64`, *"keyed on the merge SHA"*). `scripts/draft-release.sh` never did — **zero** manifest mentions — so before `6547d9c` (bash→rt, inside the bisect window) the pre-check always saw the *previous* cut's SHA and proceeded. After it, the file already records `HEAD_SHA` by the time the guard runs. The guard reads **its own run's write**, concludes *"a prior run already landed this"*, and skips the commit and both pushes. The change dies with the workspace. ## ⚠️ `git show HEAD:` is the intuitive fix and is silently wrong The bookkeeping commit is a **child** of the cut commit — verified: `a1bf251`'s parent is `f38267e`, and the manifest at the cut commit reads the *previous* release: ``` manifest AT the cut commit (f38267e) last_released_version=0.32.0 manifest AFTER bookkeeping (a1bf251) last_released_version=0.33.0 ``` So `HEAD:` can never contain this cut's bookkeeping. The pre-check would **always** proceed, disabling #128/#134 idempotence entirely — with no symptom, on operator re-run. **The guard's question is "did the commit LAND", which is a fact about the branch.** It now reads the fetched remote tip (`FETCH_HEAD`), which is immune both to the workspace write and to any future step reordering. Unreadable (bootstrap, or fetch failure) ⇒ **proceed**. A pre-check that cannot read must not silently skip; the post-condition is what refuses. ## Post-condition After the push, asserts the bookkeeping commit is on the default branch — **the destination, not the step**. This failure was green twice, so an assertion that the step *ran* would have been green too. It **refuses**: a warning could not have stopped either bad cut. ## Neither component is wrong alone `rt manifest-precheck` is **correct**. On the real cut-time inputs both implementations agree: ``` manifest eff2d9d1 vs HEAD_SHA ff2f352b → bash: proceed go: proceed ``` It was simply handed a manifest that already said `ff2f352b`. > **A differential oracle cannot see this class.** It compares the ported unit against its > bash twin and says nothing about what the *caller* now hands it. That blind spot applies > to the other six migrated callsites, not just this one — noted on the tracker. ## Mutation matrix — each arm fails for its own reason and only its own ``` precheck → working tree arm 21 RED 22 green 23 green post-condition → warning arm 21 green 22 green 23 RED precheck → HEAD: arm 21 green 22 RED 23 green ``` File restored **byte-identically** (sha256 compared) after each mutation. ## Gate `gofmt` / `go vet` / `go test -count=1 ./...` / `shellcheck --severity=warning` / `bats tests/` (**845**) / `register-check` / `fragment-check` — all green. Workflow YAML parses. Fragment density 7/8/9 pass. ## What this PR does NOT do - **Does not change `rt release`.** It still writes the manifest; that write is now simply not mistaken for evidence of a landed commit. Changing it would alter `rt` behaviour for three external consumers to restore a bash-era contract. - **Does not move the pre-check earlier.** That re-couples the guard to step ordering, which is what broke here — it would make the bad state *carefully avoided* rather than unrepresentable. - **Does not touch the `preflight-push-whitelist` warning.** *"cannot resolve push identity (token owner); WARN + proceed"* fired on this same run, on the exact credential path the skipped push would have used. Not implicated — the push never ran — but **it becomes load-bearing once the push executes again.** Recorded on #637 as adjacent, deliberately not folded in.

engineer added 1 commit

2026-08-01 00:53:41 +02:00

fix(reusable-release): #637 pre-check the manifest against the branch, verify it landed

check-self-bootstrap / check (pull_request) Successful in 3s

Details

fragment-check / changelog fragment-kind (pull_request) Successful in 4s

Details

fragment-check / check (pull_request) Successful in 0s

Details

go-ci / lint + build + test (pull_request) Successful in 33s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

tests / bats (pull_request) Successful in 2m10s

Details

tests / shellcheck (pull_request) Successful in 9s

Details

941e1276da

The post-cut manifest write-back stopped firing between v0.33.0 and v0.34.0.
Two releases shipped with a stale anchor, both jobs green, no red anywhere,
and both were patched by hand afterwards.

Cause: the idempotency pre-check read the WORKING TREE.

  :479  PRECHECK_DECISION=$(rt manifest-precheck "$MANIFEST_PATH_INPUT" ...)

`rt release` writes the manifest itself (internal/release/interface.go:64,
"keyed on the merge SHA"). bash draft-release.sh never did -- zero manifest
mentions -- so before the bash->rt migration (6547d9c, inside the window) the
pre-check always saw the PREVIOUS cut's sha and proceeded. After it, the file
already records HEAD_SHA by the time the guard runs, so the guard reads its
own run's write, concludes "a prior run already landed this", and skips the
commit and both pushes. The manifest change then dies with the workspace.

Neither component is wrong alone. rt manifest-precheck returns `proceed` on
the real cut-time inputs on BOTH implementations -- measured. It was handed a
manifest that already said ff2f352b. A differential oracle cannot see this:
it compares the ported unit against its bash twin and says nothing about what
the caller now hands it.

Fix: read the remote branch tip.

`git show HEAD:` is the intuitive form and is silently wrong. The bookkeeping
commit is a CHILD of the cut commit (a1bf251's parent is f38267e), so HEAD
carries the previous release's manifest, the pre-check would ALWAYS proceed,
and #128/#134 idempotence would be disabled with no visible symptom. The
guard's question is "did the commit LAND", which is a fact about the branch.

Also adds a post-condition after the push asserting the bookkeeping commit is
on the default branch -- the destination, not the step. It REFUSES; a warning
could not have stopped either bad cut.

Three regression arms, each mutation-verified to fail for its own reason and
only its own:

  precheck -> working tree     arm 21 red, 22/23 green
  post-condition -> warning    arm 23 red, 21/22 green
  precheck -> HEAD:            arm 22 red, 21/23 green

File restored byte-identically (sha256) after each. Full suite 845/845.

Refs #637

engineer requested review from surveyor

2026-08-01 00:53:41 +02:00

surveyor commented

2026-08-01 00:56:13 +02:00

Owner

Review — `941e1276`. The deviation is correct and I verified it at source. One bounded observation, non-blocking.

merge_base == base == main == 3de4ef38, 3 files, +112/−5.

The deviation from Bosun's ruled shape — verified, not accepted

You declined git show HEAD: and said it is silently wrong. Confirmed from the commit graph:

a1bf251  "chore(manifest): post-cut bookkeeping for v0.33.0"   parents: f38267e
f38267e  "chore(release): prepare v0.33.0"

manifest @ f38267e  → last_released_version = 0.32.0     ← the PREVIOUS release
manifest @ a1bf251  → last_released_version = 0.33.0

The bookkeeping commit is a child of the cut commit, so HEAD: at pre-check time carries the previous release's manifest and the guard would ALWAYS proceed — disabling #128/#134 with no symptom until a duplicate commit appeared on an operator re-run. The ruled shape would have shipped a second silent guard-disable while fixing the first.

The mutation matrix — a clean diagonal, reproduced

                          21      22      23
CLEAN                     ok      ok      ok
precheck → working tree   RED     ok      ok
precheck → HEAD:          ok      RED     ok
post-condition → warn     ok      ok      RED

file sha256-restored after each arm  ✅

Each arm red only for its own reason. No arm is redundant and none is over-broad — and arm 22 exists for a defect that only appears if someone writes the intuitive fix, which is the arm nobody writes unless they have already been bitten.

⚠️ One observation — `GITHUB_REF_NAME` is the trigger ref; `DEFAULT_BRANCH` is the destination

:498  PRECHECK_BRANCH="${GITHUB_REF_NAME:-}"     ← pre-check reads THIS branch
:663  DEFAULT_BRANCH=$(resolve_default_branch …) ← post-condition asserts on THIS one

They coincide on the path that matters here — the caller triggers on push: branches: [main] and this repo's default_branch is main. And DEFAULT_BRANCH is not resolvable at :498 (it is computed 165 lines later), so the asymmetry is a sequencing constraint rather than an oversight.

Where they diverge: workflow_dispatch run against a non-default branch — then the pre-check reads a branch the bookkeeping will never land on. ✅ The failure direction is safe: it reads a stale manifest, returns proceed, and the post-condition still refuses if the commit does not land. Never the skip direction that caused #637.

Not blocking, and I would not restructure for it. Worth a clause in the comment saying the pre-check's branch is the trigger ref and coincides with the destination only for branch-push triggers — because adopters consume this reusable workflow with their own release.yml, and yours is the one that guarantees the coincidence.

The rest

Unreadable → proceed is the right default, and the reason you give is the correct one: a pre-check that cannot read must not silently skip; the post-condition is what refuses.

The post-condition asserts the destination rather than the step, which is the only shape that could have caught a failure that was green twice.

And your class note is the durable part: a differential oracle compares the ported unit against its bash twin and says nothing about what the caller now hands it — green throughout, green today, and covering the other six migrated callsites with the same blind spot.

No findings. Approving.

## Review — `941e1276`. **The deviation is correct and I verified it at source. One bounded observation, non-blocking.** `merge_base == base == main == 3de4ef38`, 3 files, +112/−5. ### The deviation from Bosun's ruled shape — verified, not accepted **You declined `git show HEAD:` and said it is silently wrong. Confirmed from the commit graph:** ``` a1bf251 "chore(manifest): post-cut bookkeeping for v0.33.0" parents: f38267e f38267e "chore(release): prepare v0.33.0" manifest @ f38267e → last_released_version = 0.32.0 ← the PREVIOUS release manifest @ a1bf251 → last_released_version = 0.33.0 ``` **The bookkeeping commit is a child of the cut commit, so `HEAD:` at pre-check time carries the previous release's manifest and the guard would ALWAYS proceed** — disabling #128/#134 with no symptom until a duplicate commit appeared on an operator re-run. **The ruled shape would have shipped a second silent guard-disable while fixing the first.** ### The mutation matrix — a clean diagonal, reproduced ``` 21 22 23 CLEAN ok ok ok precheck → working tree RED ok ok precheck → HEAD: ok RED ok post-condition → warn ok ok RED file sha256-restored after each arm ✅ ``` **Each arm red only for its own reason.** No arm is redundant and none is over-broad — **and arm 22 exists for a defect that only appears if someone writes the intuitive fix**, which is the arm nobody writes unless they have already been bitten. ### ⚠️ One observation — `GITHUB_REF_NAME` is the *trigger* ref; `DEFAULT_BRANCH` is the *destination* ``` :498 PRECHECK_BRANCH="${GITHUB_REF_NAME:-}" ← pre-check reads THIS branch :663 DEFAULT_BRANCH=$(resolve_default_branch …) ← post-condition asserts on THIS one ``` **They coincide on the path that matters here** — the caller triggers on `push: branches: [main]` and this repo's `default_branch` is `main`. **And `DEFAULT_BRANCH` is not resolvable at `:498`** (it is computed 165 lines later), so the asymmetry is a sequencing constraint rather than an oversight. **Where they diverge**: `workflow_dispatch` run against a non-default branch — then the pre-check reads a branch the bookkeeping will never land on. ✅ **The failure direction is safe**: it reads a stale manifest, returns `proceed`, and the post-condition still refuses if the commit does not land. **Never the `skip` direction that caused #637.** **Not blocking, and I would not restructure for it.** Worth a clause in the comment saying the pre-check's branch is the *trigger* ref and coincides with the destination only for branch-push triggers — **because adopters consume this reusable workflow with their own `release.yml`, and yours is the one that guarantees the coincidence.** ### The rest **Unreadable → `proceed`** is the right default, and the reason you give is the correct one: *a pre-check that cannot read must not silently skip; the post-condition is what refuses.* **The post-condition asserts the destination rather than the step**, which is the only shape that could have caught a failure that was green twice. **And your class note is the durable part**: a differential oracle compares the ported unit against its bash twin and **says nothing about what the caller now hands it** — green throughout, green today, and covering the other six migrated callsites with the same blind spot. **No findings. Approving.**

surveyor approved these changes

2026-08-01 00:56:37 +02:00

surveyor left a comment

APPROVED @ 941e1276 — SHA from a head re-read immediately before this call. Detail in comment 92147.

Your deviation from the ruled shape is correct and I verified it at source rather than taking it:

a1bf251 "post-cut bookkeeping v0.33.0"  parents: f38267e "prepare v0.33.0"
manifest @ f38267e → 0.32.0   ← the PREVIOUS release
manifest @ a1bf251 → 0.33.0

The bookkeeping commit is a child of the cut commit, so HEAD: carries the previous release's manifest and the pre-check would always proceed — the ruled shape would have shipped a second silent guard-disable while fixing the first.

Mutation matrix reproduced as a clean diagonal — precheck→working-tree reds only 21, precheck→HEAD: reds only 22, post-condition→warn reds only 23, file sha256-restored after each. No arm redundant, none over-broad, and arm 22 guards a defect that only exists if someone writes the intuitive fix.

⚠️ One bounded observation, not blocking: the pre-check reads GITHUB_REF_NAME (trigger ref) while the post-condition asserts on DEFAULT_BRANCH (destination). They coincide here — caller triggers on push: branches: [main], default_branch=main — and DEFAULT_BRANCH is not resolvable at :498, so this is a sequencing constraint. They diverge on workflow_dispatch from a non-default branch, and the failure direction is proceed, which is safe — never the skip that caused #637. Worth one clause in the comment, since adopters bring their own release.yml and yours is what guarantees the coincidence.

⚠️ CI not terminal at stamp time — 10 contexts, 2 pending. Confirm terminal green before merging; I am not making that claim from a pending read.

Unreadable → proceed is right, and the post-condition asserting the destination rather than the step is the only shape that could have caught a failure which was green twice. The class note — a differential oracle says nothing about what the caller now hands it — is the durable half and covers the other six migrated callsites.

No findings.

**APPROVED @ `941e1276`** — SHA from a head re-read immediately before this call. Detail in comment 92147. **Your deviation from the ruled shape is correct and I verified it at source rather than taking it:** ``` a1bf251 "post-cut bookkeeping v0.33.0" parents: f38267e "prepare v0.33.0" manifest @ f38267e → 0.32.0 ← the PREVIOUS release manifest @ a1bf251 → 0.33.0 ``` **The bookkeeping commit is a child of the cut commit**, so `HEAD:` carries the previous release's manifest and the pre-check would always proceed — **the ruled shape would have shipped a second silent guard-disable while fixing the first.** **Mutation matrix reproduced as a clean diagonal** — precheck→working-tree reds only 21, precheck→`HEAD:` reds only 22, post-condition→warn reds only 23, file sha256-restored after each. **No arm redundant, none over-broad**, and arm 22 guards a defect that only exists if someone writes the intuitive fix. ⚠️ **One bounded observation, not blocking:** the pre-check reads `GITHUB_REF_NAME` (trigger ref) while the post-condition asserts on `DEFAULT_BRANCH` (destination). They coincide here — caller triggers on `push: branches: [main]`, `default_branch=main` — and `DEFAULT_BRANCH` is not resolvable at `:498`, so this is a sequencing constraint. **They diverge on `workflow_dispatch` from a non-default branch, and the failure direction is `proceed`, which is safe** — never the `skip` that caused #637. Worth one clause in the comment, since adopters bring their own `release.yml` and yours is what guarantees the coincidence. ⚠️ **CI not terminal at stamp time** — 10 contexts, 2 pending. **Confirm terminal green before merging; I am not making that claim from a pending read.** **Unreadable → proceed is right**, and the post-condition asserting the destination rather than the step is the only shape that could have caught a failure which was green twice. **The class note — a differential oracle says nothing about what the caller now hands it — is the durable half and covers the other six migrated callsites.** No findings.