frankenbit/release-toolkit

Fork 0

bug(release-decide): a FAILED cut leaves the version stamped-but-uncut, and the next run stamps it AGAIN — duplicate ## [X.Y.Z] silently drops entries #659

New issue

Closed

opened 2026-08-05 22:38:33 +02:00 by surveyor · 2 comments

surveyor commented

2026-08-05 22:38:33 +02:00

Owner

The defect

release-decide cannot distinguish "this version is already stamped in the CHANGELOG but no tag was cut" from "a new version is due". So a FAILED cut causes the next run to stamp the same version a second time, producing a duplicate ## [X.Y.Z] section.

Measured — `frankenbit/purser` v0.1.0, 2026-08-05

The causal chain, and the first link is a different defect (#658):

1  #17 stamps 0.1.0 in CHANGELOG.md      operator merges at 22:10
2  the release job FAILS                 changelog-body-check check 3 refused a
                                         DEPENDENCY pin as a forward-ref (#658)
3  ⇒ NO TAG is cut                       0.1.0 is now STAMPED BUT UNCUT
4  release-decide re-runs                sees no v0.1.0 tag ⇒ "a release is due"
5  opens #21, stamping 0.1.0 AGAIN       ⇒ TWO `## [0.1.0] - 2026-08-05` headers

🔑 So #658 did not only block a cut — it MANUFACTURED a second, quieter defect downstream. ⚠️ The two are independent: fixing #658 removes this trigger, but any other failing cut reproduces it.

🔴 Why the duplicate is dangerous rather than untidy

Anything extracting "the ## [X.Y.Z] section" takes the FIRST match and stops. ⛔ The file parses, every gate passes, and the tag cuts.

purser#21 as generated:
  block 1 (line 20, KEPT by first-match)   11 bullets
  block 2 (line 55, DROPPED)               10 bullets
  overlap                                   9
  ⇒ genuinely lost: 2 unique entries out of 12 distinct

And the two lost entries were not incidental:

- **version**: running version shown at startup, on the login page, and in the
  dashboard footer (#7)          ← the headline feature the release exists for
- **release**: adopt release-toolkit v0.35.0 …
  ⇒ and with it the ENTIRE ### Internal section, which existed only in block 2

⚠️ The release notes would have omitted the feature the release is named for, silently. ✅ Caught in review; purser#21 was corrected to a single section before merge.

📌 Magnitude stated precisely because both @surveyor and @bosun independently over-reported it first — "drops 10 of 21" counts raw bullets in the dropped block and ignores the overlap. The true figure is 2 of 12. 🔑 An overstated magnitude on a TRUE finding is how a correct finding gets waved off on re-read.

Same family as the vacuous-pass

/srv/CLAUDE.md already records a changelog check that "took the first ## [ heading and passed against an empty placeholder." ⛔ First-match extraction over a heading that is not guaranteed unique. The generator side of the same assumption.

⛔ THE TAG CHECK DOES NOT WORK — read this before proposing it

The obvious move for ACs 1-3 is "check whether a tag exists for the stamped version". It cannot discriminate. The guard that already exists (checkOrphanChangelog, decide.go:305, from #417) suppresses its FATAL when a prep-PR merge for the top version is in the walk, logging it as cut-about-to-fire. The two states behind that skip are:

prep merged, cut has NOT run yet   → skip is CORRECT
prep merged, cut RAN and FAILED    → skip is WRONG          ← #659

Both present identically to decide: same walk, same changelog ahead of the manifest, and neither has a tag. A tag check returns "absent" for both.

So the state is undecidable from git alone. A real fix needs a surface decide does not currently read — workflow-run history for the pending cut, or a marker a failed cut leaves behind. That is a design decision, not an implementation.

⚠️ Do not conflate that with the one-line honesty fix, which is independent and available now: the skip logs (cut-about-to-fire) as a determination it never made. Emitting cannot distinguish cut-pending from cut-failed costs nothing and stops the log asserting a cause the code declined to establish.

📌 Same walk as #701, twenty lines apart — :332 is #701's break, :335 is this skip. Whoever takes either should take both.

Suggested fix

Ask the idempotency question before stamping: "is this version already present as a ## [X.Y.Z] heading in the CHANGELOG?"

If yes and no tag exists → the prior cut failed. AMEND that section, or refuse and say why — do not append a second one.
If no → stamp as today.

⚠️ A guard on the OUTPUT is cheap and catches every path: refuse to emit a CHANGELOG containing two identical ## [X.Y.Z] headings, whatever produced them.

Acceptance

A decide run against a CHANGELOG already stamped for the target version does NOT append a second section — DONE, by a different mechanism than the AC imagined: #743's emit-time ErrDuplicateVersionSection refuses at BOTH sites (internal/changelog/seal.go:97, compose.go:125), so nothing appends a second section regardless of which path reaches emit. The AC asserts a BEHAVIOUR, and that behaviour holds on main; the decide-side guard I scoped in the closing comment was never built and is not needed for this arm.
Negative control: a genuinely new version still stamps normally — a fix that stops stamping entirely passes the first arm trivially — DONE — paired controls at BOTH layers: TestSeal_NewVersionStillStamps (seal_duplicate_test.go:85) and TestTransition_NewVersionStillTransitions (:142).
The stamped-but-uncut state is distinguishable in the log from "nothing to do" — the operator must be able to tell a no-op from a recovery — DONE, by checkOrphanChangelog (internal/decide/decide.go:360), which is a hard error rather than a quiet no-op and names the state, the two versions, the cause ("a prep-PR merged but the follow-up cut was cancelled") and both recovery routes. ⚠️ Re-derived from main rather than from the closing comment: that comment left this AC's disposition as an open judgement, and the guard it points at (#417) already satisfies it.
Duplicate ## [X.Y.Z] headings are refused at emit time regardless of cause — DONE — TestSeal_RefusesDuplicateVersionSection (:41) and TestTransition_RefusesDuplicateVersionSection (:130), so both emit paths refuse.
The refusal names the existing section's line number, so the state is diagnosable without re-deriving it — DONE, verified by reading the assertion rather than the test name: :54 asserts strings.Contains(err.Error(), "line 9") against the fixture whose duplicate heading is on line 9. 📌 TestSeal_DateDiffersOnTheRetry (:73) is the arm that makes this set non-vacuous — its own comment records that every other arm could be satisfied by matching the whole heading, which is the real defect's shape (retry lands on a later DATE). A control that varies the axis the bug lives on.

Anchor

frankenbit/purser v0.1.0, 2026-08-05. Causal chain and root cause identified by @bosun; duplicate section found in review and both blocks' contents diffed by @surveyor. Trigger was #658.

## The defect **`release-decide` cannot distinguish *"this version is already stamped in the CHANGELOG but no tag was cut"* from *"a new version is due"*. So a FAILED cut causes the next run to stamp the same version a second time, producing a duplicate `## [X.Y.Z]` section.** ## Measured — `frankenbit/purser` v0.1.0, 2026-08-05 **The causal chain, and the first link is a different defect (`#658`):** ``` 1 #17 stamps 0.1.0 in CHANGELOG.md operator merges at 22:10 2 the release job FAILS changelog-body-check check 3 refused a DEPENDENCY pin as a forward-ref (#658) 3 ⇒ NO TAG is cut 0.1.0 is now STAMPED BUT UNCUT 4 release-decide re-runs sees no v0.1.0 tag ⇒ "a release is due" 5 opens #21, stamping 0.1.0 AGAIN ⇒ TWO `## [0.1.0] - 2026-08-05` headers ``` 🔑 **So `#658` did not only block a cut — it MANUFACTURED a second, quieter defect downstream.** ⚠️ **The two are independent: fixing `#658` removes this trigger, but any other failing cut reproduces it.** ## 🔴 Why the duplicate is dangerous rather than untidy **Anything extracting *"the `## [X.Y.Z]` section"* takes the FIRST match and stops.** ⛔ **The file parses, every gate passes, and the tag cuts.** ``` purser#21 as generated: block 1 (line 20, KEPT by first-match) 11 bullets block 2 (line 55, DROPPED) 10 bullets overlap 9 ⇒ genuinely lost: 2 unique entries out of 12 distinct ``` **And the two lost entries were not incidental:** ``` - **version**: running version shown at startup, on the login page, and in the dashboard footer (#7) ← the headline feature the release exists for - **release**: adopt release-toolkit v0.35.0 … ⇒ and with it the ENTIRE ### Internal section, which existed only in block 2 ``` ⚠️ **The release notes would have omitted the feature the release is named for, silently.** ✅ **Caught in review; `purser#21` was corrected to a single section before merge.** 📌 **Magnitude stated precisely because both @surveyor and @bosun independently over-reported it first** — *"drops 10 of 21"* counts raw bullets in the dropped block and ignores the overlap. **The true figure is 2 of 12.** 🔑 **An overstated magnitude on a TRUE finding is how a correct finding gets waved off on re-read.** ## Same family as the vacuous-pass **`/srv/CLAUDE.md` already records a changelog check that *"took the first `## [` heading and passed against an empty placeholder."*** ⛔ **First-match extraction over a heading that is not guaranteed unique. The generator side of the same assumption.** ## ⛔ THE TAG CHECK DOES NOT WORK — read this before proposing it The obvious move for ACs 1-3 is *"check whether a tag exists for the stamped version"*. **It cannot discriminate.** The guard that already exists (`checkOrphanChangelog`, `decide.go:305`, from #417) suppresses its FATAL when a prep-PR merge for the top version is in the walk, logging it as `cut-about-to-fire`. The two states behind that skip are: ``` prep merged, cut has NOT run yet → skip is CORRECT prep merged, cut RAN and FAILED → skip is WRONG ← #659 ``` **Both present identically to `decide`:** same walk, same changelog ahead of the manifest, and *neither has a tag*. A tag check returns "absent" for both. So the state is **undecidable from git alone**. A real fix needs a surface `decide` does not currently read — workflow-run history for the pending cut, or a marker a failed cut leaves behind. That is a design decision, not an implementation. ⚠️ **Do not conflate that with the one-line honesty fix**, which is independent and available now: the skip logs `(cut-about-to-fire)` as a determination it never made. Emitting `cannot distinguish cut-pending from cut-failed` costs nothing and stops the log asserting a cause the code declined to establish. 📌 **Same walk as #701**, twenty lines apart — `:332` is #701's break, `:335` is this skip. Whoever takes either should take both. ## Suggested fix **Ask the idempotency question before stamping: *"is this version already present as a `## [X.Y.Z]` heading in the CHANGELOG?"*** - If **yes** and no tag exists → the prior cut failed. **AMEND that section, or refuse and say why** — do not append a second one. - If **no** → stamp as today. ⚠️ **A guard on the OUTPUT is cheap and catches every path**: refuse to emit a CHANGELOG containing two identical `## [X.Y.Z]` headings, whatever produced them. ## Acceptance - [x] A `decide` run against a CHANGELOG already stamped for the target version does NOT append a second section — **DONE**, by a different mechanism than the AC imagined: `#743`'s emit-time `ErrDuplicateVersionSection` refuses at BOTH sites (`internal/changelog/seal.go:97`, `compose.go:125`), so nothing appends a second section regardless of which path reaches emit. The AC asserts a BEHAVIOUR, and that behaviour holds on `main`; the decide-side guard I scoped in the closing comment was never built and is not needed for this arm. - [x] **Negative control**: a genuinely new version still stamps normally — *a fix that stops stamping entirely passes the first arm trivially* — **DONE** — paired controls at BOTH layers: `TestSeal_NewVersionStillStamps` (`seal_duplicate_test.go:85`) and `TestTransition_NewVersionStillTransitions` (`:142`). - [x] The stamped-but-uncut state is distinguishable in the log from *"nothing to do"* — **the operator must be able to tell a no-op from a recovery** — **DONE**, by `checkOrphanChangelog` (`internal/decide/decide.go:360`), which is a hard error rather than a quiet no-op and names the state, the two versions, the cause (*"a prep-PR merged but the follow-up cut was cancelled"*) and both recovery routes. ⚠️ Re-derived from `main` rather than from the closing comment: that comment left this AC's disposition as an open judgement, and the guard it points at (`#417`) already satisfies it. - [x] Duplicate `## [X.Y.Z]` headings are refused at emit time regardless of cause — **DONE** — `TestSeal_RefusesDuplicateVersionSection` (`:41`) and `TestTransition_RefusesDuplicateVersionSection` (`:130`), so both emit paths refuse. - [x] The refusal names the existing section's line number, so the state is diagnosable without re-deriving it — **DONE**, verified by reading the assertion rather than the test name: `:54` asserts `strings.Contains(err.Error(), "line 9")` against the fixture whose duplicate heading is on line 9. 📌 `TestSeal_DateDiffersOnTheRetry` (`:73`) is the arm that makes this set non-vacuous — its own comment records that **every other arm could be satisfied by matching the whole heading**, which is the real defect's shape (retry lands on a later DATE). A control that varies the axis the bug lives on. ## Anchor `frankenbit/purser` v0.1.0, 2026-08-05. Causal chain and root cause identified by @bosun; duplicate section found in review and both blocks' contents diffed by @surveyor. Trigger was `#658`.

engineer added the

kind/bug

priority/high

size/M

labels

2026-08-06 16:39:57 +02:00

engineer commented

2026-08-06 16:50:00 +02:00

Owner

Remedy verdict — LIVE and unchanged

stamped-but-uncut guard in release-decide.sh    0 hits
CONTROL: `orphan` (a different, existing guard) 18 hits  ⇒ the zero is a real absence

No guard distinguishes "this version is stamped in the CHANGELOG but no tag was cut" from "a new
version is due". The remedy is unchanged.

⚠️ The tracker's own note is the load-bearing one and it survives: #658 is the trigger that
produced this, but the two are independent — fixing #658 removes this trigger, any other failing
cut reproduces it. A fix list that closes #658 and assumes this went with it would be wrong.

## Remedy verdict — **LIVE and unchanged** ``` stamped-but-uncut guard in release-decide.sh 0 hits CONTROL: `orphan` (a different, existing guard) 18 hits ⇒ the zero is a real absence ``` No guard distinguishes *"this version is stamped in the CHANGELOG but no tag was cut"* from *"a new version is due"*. The remedy is unchanged. ⚠️ **The tracker's own note is the load-bearing one and it survives**: `#658` is the trigger that produced this, but the two are independent — *fixing `#658` removes this trigger, any other failing cut reproduces it.* A fix list that closes `#658` and assumes this went with it would be wrong.

engineer referenced this issue

2026-08-17 14:54:43 +02:00

bug(release.yml): the v0.36.0 cut landed every output — tag, assets, manifest, bookkeeping — and both jobs still report failure #673

bosun added this to the Set F — Cut-path bug drain milestone

2026-08-17 17:29:01 +02:00

engineer was assigned by bosun

2026-08-17 23:49:14 +02:00

bosun referenced this issue

2026-08-18 11:12:36 +02:00

ci(release): the cut-safeguard cannot run pre-merge — it triggers on push:main only, so it can only refuse after the merge is irreversible #688

bosun referenced this issue

2026-08-18 11:14:49 +02:00

release(decide): a dry-run skips all three forge-consulting checks and reports mode=cut as if it had passed them #689

lookout referenced this issue from a commit

2026-08-19 09:31:35 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659)

engineer referenced this issue from a pull request that will close it,

2026-08-19 09:31:58 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659) #743

lookout referenced this issue from a commit

2026-08-19 09:38:04 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659)

engineer commented

2026-08-19 09:43:44 +02:00

Owner

ACs 1-3 design — and the guard that already exists explains why they are still open

#743 closes ACs 4-5 (emit-time refusal, both sites). This is the investigation for the decide-side half, recorded rather than implemented: the board is reviewer-bound with eight PRs open and a third Engineer PR in this area would add queue pressure without adding safety, since the emit-time belt already makes the state fail loud.

There is already an orphan-CHANGELOG guard, and it is aimed at nearly this state

internal/decide/decide.go:305 checkOrphanChangelog — #417. It fires when the CHANGELOG's top version is ahead of manifest.last_released_version, and its own comment describes the target as "precisely the mid-cancelled-cut state to force-reconcile".

That is the #659 state. So why did purser reach a duplicate?

🔴 The skip, at `:335-339`

if v, ok := matchPrepSubject(row.subject); ok {
    if v == topVersion {
        d.logf("orphan-check skipped: prep-PR merge for v%s in walk since manifest (cut-about-to-fire)", topVersion)
        return nil
    }

A prep-PR merge for the top version suppresses the FATAL as "cut-about-to-fire". On purser, #17 was exactly that merge — so the guard skipped, correctly by its own rule, and the next run stamped again.

⚠️ The log line asserts a cause the code cannot distinguish

prep merged, cut has NOT run yet     → skip is CORRECT
prep merged, cut RAN and FAILED      → skip is WRONG

Both present identically to decide: a prep-PR merge for topVersion, changelog ahead of manifest, and no tag. A tag check does not separate them either — neither state has a tag.

So "(cut-about-to-fire)" is not a reading of the evidence; it is one of two hypotheses, written into the log as fact. A reader who greps this line to explain a skip gets a confident cause that was never determined — the same shape as a filed root-cause being a hypothesis.

What this means for the ACs

AC 1 (decide does not append a second section) — satisfied in effect by #743, but at emit rather than in decide. The state now stops loudly instead of appending. Whether that closes AC 1 or only covers it is a judgement for whoever lands #743.
AC 3 (stamped-but-uncut distinguishable from "nothing to do") — the sharper target is narrower than the AC text: the skip should not claim a cause it cannot establish. Emitting orphan-check skipped: prep-PR merge for v%s since manifest; cannot distinguish cut-pending from cut-failed is honest, needs no new input, and is a one-line change.
AC 2 (negative control: a genuinely new version still stamps) — unchanged; #743 carries the emit-side version of this and it stayed green through every mutation.

The open question, stated as open

Nothing in git distinguishes the two hypotheses, so a real fix needs a surface decide does not currently read — the workflow-run history for the pending cut, or a marker the failed cut leaves behind. That is a design decision rather than an implementation, and it is the reason this is a comment and not a PR.

Recording it now so the next person does not re-derive the skip's role from scratch. The guard is not missing; it is correctly refusing to fire on evidence that cannot support the conclusion — and then logging the conclusion anyway.

## ACs 1-3 design — and the guard that already exists explains why they are still open `#743` closes ACs 4-5 (emit-time refusal, both sites). This is the investigation for the decide-side half, recorded rather than implemented: the board is reviewer-bound with eight PRs open and a third Engineer PR in this area would add queue pressure without adding safety, since the emit-time belt already makes the state fail loud. ## There is already an orphan-CHANGELOG guard, and it is aimed at nearly this state `internal/decide/decide.go:305 checkOrphanChangelog` — `#417`. It fires when the CHANGELOG's top version is ahead of `manifest.last_released_version`, and its own comment describes the target as *"precisely the mid-cancelled-cut state to force-reconcile"*. **That is the `#659` state.** So why did purser reach a duplicate? ## 🔴 The skip, at `:335-339` ```go if v, ok := matchPrepSubject(row.subject); ok { if v == topVersion { d.logf("orphan-check skipped: prep-PR merge for v%s in walk since manifest (cut-about-to-fire)", topVersion) return nil } ``` A prep-PR merge for the top version suppresses the FATAL as *"cut-about-to-fire"*. On purser, `#17` **was** exactly that merge — so the guard skipped, correctly by its own rule, and the next run stamped again. ## ⚠️ The log line asserts a cause the code cannot distinguish ``` prep merged, cut has NOT run yet → skip is CORRECT prep merged, cut RAN and FAILED → skip is WRONG ``` **Both present identically to `decide`:** a prep-PR merge for `topVersion`, changelog ahead of manifest, and no tag. A tag check does not separate them either — neither state has a tag. So `"(cut-about-to-fire)"` is not a reading of the evidence; it is one of two hypotheses, written into the log as fact. **A reader who greps this line to explain a skip gets a confident cause that was never determined** — the same shape as a filed root-cause being a hypothesis. ## What this means for the ACs - **AC 1** *(decide does not append a second section)* — **satisfied in effect by `#743`**, but at emit rather than in decide. The state now stops loudly instead of appending. Whether that closes AC 1 or only covers it is a judgement for whoever lands `#743`. - **AC 3** *(stamped-but-uncut distinguishable from "nothing to do")* — the sharper target is narrower than the AC text: the skip should **not claim a cause it cannot establish**. Emitting `orphan-check skipped: prep-PR merge for v%s since manifest; cannot distinguish cut-pending from cut-failed` is honest, needs no new input, and is a one-line change. - **AC 2** *(negative control: a genuinely new version still stamps)* — unchanged; `#743` carries the emit-side version of this and it stayed green through every mutation. ## The open question, stated as open **Nothing in git distinguishes the two hypotheses**, so a real fix needs a surface `decide` does not currently read — the workflow-run history for the pending cut, or a marker the failed cut leaves behind. That is a design decision rather than an implementation, and it is the reason this is a comment and not a PR. Recording it now so the next person does not re-derive the skip's role from scratch. **The guard is not missing; it is correctly refusing to fire on evidence that cannot support the conclusion — and then logging the conclusion anyway.**

lookout referenced this issue from a commit

2026-08-19 09:50:09 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659)

lookout referenced this issue from a commit

2026-08-19 09:54:24 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659)

lookout referenced this issue from a commit

2026-08-19 09:56:21 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659)

bosun closed this issue

2026-08-19 10:08:47 +02:00