bug(changelog): Seal inserts a duplicate version heading on every re-prepare — empty sections accumulate per failed cut #691

Closed
opened 2026-08-18 11:33:20 +02:00 by bosun · 6 comments
Owner

Motivation

Every failed cut leaves an extra empty version section in CHANGELOG.md, and they accumulate.

release-prep/rolling currently carries two ## [v0.37.1] headings:

23:  ## [v0.37.1] - 2026-08-18     ← Added/Changed/Fixed/Removed all "None."
49:  ## [v0.37.1] - 2026-08-17     ← the REAL section, with the #650 + #655 content

If PR#687 merged as it stands, the published v0.37.1 changelog would read empty, with the
real content buried below a second identical heading.

⚠️ mergeable reports true for this PR. It is a content defect, not a git conflict — no
gate on the PR sees it.

Mechanism — measured, not inferred

Two prepare commits on the branch, one per cut attempt:

e5f18083  2026-08-17T21:19Z  chore(release): prepare v0.37.1   ← attempt 1, sealed the REAL section
24962684  2026-08-18T08:42Z  docs(changelog): split the #650 entry
244849b0  2026-08-18T09:01Z  chore(release): prepare v0.37.1   ← attempt 2, sealed the EMPTY one

Two halves, both verified against the source:

  1. Seal never checks whether the version already has a section. internal/release/cutter.go:227
    calls changelog.Seal(priorCL, vs, …) unconditionally; internal/changelog/seal.go:94-98 scans
    for ## [Unreleased] and inserts the new heading immediately after it. There is no existence
    test for ## [<version>] anywhere on the path.

  2. The second section is empty because the fragments were already consumed. changelog.d/ on
    release-prep/rolling holds only .keep and .template.md — attempt 1 consumed them under the
    §4 fragment-consumption atomicity property. Attempt 2 therefore composed an empty section and
    sealed it anyway.

So the defect compounds: a third attempt adds a third empty section, a fourth adds a fourth.

Why no gate catches it

The cut path has three changelog gates and none is scoped to this:

  • Gate 2 (#442 register drift)cutter.go:215 runs register.FindHits on the composed body,
    which is empty. Empty text has no drift.
  • changelog-body-check.sh — density/prose shape of a section's body.
  • compose-verify.sh — bash/Go compose equivalence.

None asks "does this file already contain a section for the version being sealed?"

Scope

  • internal/changelog/seal.go — refuse, or replace-in-place, when ## [<version>] is already present
  • Decide which: refusing is the safer default per /srv/CLAUDE.md § Mechanism design (a refusal is
    a wrong answer that costs nothing); replace-in-place is friendlier to the retry path that
    produced this. A third option is to make the empty-compose case itself refuse.
  • Whatever is chosen, the existing duplicate on release-prep/rolling needs removing by hand before
    any successful v0.37.1 cut.

⚠️ Note the interaction with fragment consumption: on a retry the fragments are legitimately gone,
so "compose produced nothing" is the EXPECTED state for a re-prepare, not an anomaly. A fix that
refuses on empty-compose must not break the ordinary re-prepare path — it needs to distinguish
"nothing to say because already sealed" from "nothing to say because no fragments were ever written."

Acceptance criteria

  • Sealing a version that already has a section in the target changelog produces a defined,
    tested outcome (refuse or replace) rather than a silent second heading
  • An arm covers the retry shape specifically: fragments consumed, compose empty, section present
  • The duplicate on release-prep/rolling is removed before the v0.37.1 cutRETIRED (deadline passed): v0.37.1 published 2026-08-18T12:27:36; six further cuts have shipped since. The gate that makes this class impossible landed in #743.
  • The PASS/refusal names what it did NOT check, per § Mechanism design — NOT MET. compose.go:124 names the version, the line and the remedy, but carries no scope note. DEFERRED → #833, which owns adding the scope note to the refusal
  • #690 — the decide-layer root cause that caused the retry in the first place
  • #688 — the cut path has no pre-merge CI coverage, which is why this surfaced only at cut time
  • Family: /srv/CLAUDE.md § Verification-instrument artifacts — a mechanism that has never run on
    the retry path has a green history that is a history of not running

Anchor

Found 2026-08-18 by Bosun while checking why PR#687 reported mergeable=true after the v0.37.1 cut
failure. Both halves of the mechanism read from source on main; the two-prepare-commit sequence read
from the branch. Not inferred from the symptom.

## Motivation **Every failed cut leaves an extra empty version section in `CHANGELOG.md`, and they accumulate.** `release-prep/rolling` currently carries **two** `## [v0.37.1]` headings: ``` 23: ## [v0.37.1] - 2026-08-18 ← Added/Changed/Fixed/Removed all "None." 49: ## [v0.37.1] - 2026-08-17 ← the REAL section, with the #650 + #655 content ``` If PR#687 merged as it stands, the published v0.37.1 changelog would read **empty**, with the real content buried below a second identical heading. ⚠️ **`mergeable` reports `true` for this PR.** It is a content defect, not a git conflict — no gate on the PR sees it. ## Mechanism — measured, not inferred Two prepare commits on the branch, one per cut attempt: ``` e5f18083 2026-08-17T21:19Z chore(release): prepare v0.37.1 ← attempt 1, sealed the REAL section 24962684 2026-08-18T08:42Z docs(changelog): split the #650 entry 244849b0 2026-08-18T09:01Z chore(release): prepare v0.37.1 ← attempt 2, sealed the EMPTY one ``` Two halves, both verified against the source: 1. **`Seal` never checks whether the version already has a section.** `internal/release/cutter.go:227` calls `changelog.Seal(priorCL, vs, …)` unconditionally; `internal/changelog/seal.go:94-98` scans for `## [Unreleased]` and inserts the new heading immediately after it. There is no existence test for `## [<version>]` anywhere on the path. 2. **The second section is empty because the fragments were already consumed.** `changelog.d/` on `release-prep/rolling` holds only `.keep` and `.template.md` — attempt 1 consumed them under the §4 fragment-consumption atomicity property. Attempt 2 therefore composed an empty section and sealed it anyway. **So the defect compounds**: a third attempt adds a third empty section, a fourth adds a fourth. ## Why no gate catches it The cut path has three changelog gates and none is scoped to this: - **Gate 2 (#442 register drift)** — `cutter.go:215` runs `register.FindHits` on the *composed body*, which is empty. Empty text has no drift. - **`changelog-body-check.sh`** — density/prose shape of a section's body. - **`compose-verify.sh`** — bash/Go compose equivalence. None asks *"does this file already contain a section for the version being sealed?"* ## Scope - `internal/changelog/seal.go` — refuse, or replace-in-place, when `## [<version>]` is already present - Decide which: **refusing** is the safer default per `/srv/CLAUDE.md` § Mechanism design (a refusal is a wrong answer that costs nothing); **replace-in-place** is friendlier to the retry path that produced this. A third option is to make the empty-compose case itself refuse. - Whatever is chosen, the existing duplicate on `release-prep/rolling` needs removing by hand before any successful v0.37.1 cut. ⚠️ **Note the interaction with fragment consumption**: on a retry the fragments are legitimately gone, so "compose produced nothing" is the EXPECTED state for a re-prepare, not an anomaly. A fix that refuses on empty-compose must not break the ordinary re-prepare path — it needs to distinguish *"nothing to say because already sealed"* from *"nothing to say because no fragments were ever written."* ## Acceptance criteria - [x] Sealing a version that already has a section in the target changelog produces a defined, tested outcome (refuse or replace) rather than a silent second heading - [x] An arm covers the retry shape specifically: fragments consumed, compose empty, section present - [x] ~~The duplicate on `release-prep/rolling` is removed before the v0.37.1 cut~~ — **RETIRED (deadline passed):** v0.37.1 published 2026-08-18T12:27:36; six further cuts have shipped since. The gate that makes this class impossible landed in #743. - [x] The PASS/refusal names what it did NOT check, per § Mechanism design — **NOT MET.** `compose.go:124` names the version, the line and the remedy, but carries no scope note. **DEFERRED → #833**, which owns adding the scope note to the refusal ## Related - `#690` — the decide-layer root cause that caused the retry in the first place - `#688` — the cut path has no pre-merge CI coverage, which is why this surfaced only at cut time - Family: `/srv/CLAUDE.md` § Verification-instrument artifacts — *a mechanism that has never run on the retry path has a green history that is a history of not running* ## Anchor Found 2026-08-18 by Bosun while checking why PR#687 reported `mergeable=true` after the v0.37.1 cut failure. Both halves of the mechanism read from source on `main`; the two-prepare-commit sequence read from the branch. Not inferred from the symptom.
Author
Owner

Confirmed the consequence rather than leaving it as an assumption.

parse.go:156 SectionContent takes the first matching heading (break at :171) and stops at
the next ## [ (:177). With the empty 08-18 section sitting above the real 08-17 one, the
extraction returns the empty one and never reaches the content.

Measured against the actual branch bytes:

extract v0.37.1  ->  Added/Changed/Fixed/Removed/Deprecated/Upgrade all "None."
extract v0.37.0  ->  real content (control — the instrument is not returning empty for everything)

So the v0.37.1 release body publishes empty if this merges. Not a cosmetic duplicate.

One caveat for anyone re-running this: my first simulation was wrong and printed both sections
concatenated. The awk rule that set the start flag also matched the second heading and its
next skipped the exit rule, so it never terminated where the Go code does. The corrected form
guards the start rule with !started. Worth stating because the wrong output looks plausible —
it shows the real content present, which reads as "the duplicate is harmless".

Also noting without chasing it: Parse (:44-91) collects all sections, so it yields two entries
both named v0.37.1. Any consumer keyed on that list sees a duplicate rather than a shadowed
section. Not investigated.

Confirmed the consequence rather than leaving it as an assumption. `parse.go:156 SectionContent` takes the **first** matching heading (`break` at :171) and stops at the next `## [` (:177). With the empty 08-18 section sitting above the real 08-17 one, the extraction returns the empty one and never reaches the content. Measured against the actual branch bytes: ``` extract v0.37.1 -> Added/Changed/Fixed/Removed/Deprecated/Upgrade all "None." extract v0.37.0 -> real content (control — the instrument is not returning empty for everything) ``` So the v0.37.1 release body publishes empty if this merges. Not a cosmetic duplicate. One caveat for anyone re-running this: my first simulation was wrong and printed both sections concatenated. The awk rule that set the start flag also matched the *second* heading and its `next` skipped the exit rule, so it never terminated where the Go code does. The corrected form guards the start rule with `!started`. Worth stating because the wrong output looks plausible — it shows the real content present, which reads as "the duplicate is harmless". Also noting without chasing it: `Parse` (:44-91) collects all sections, so it yields two entries both named v0.37.1. Any consumer keyed on that list sees a duplicate rather than a shadowed section. Not investigated.
Author
Owner

Open question resolved: the cut does NOT re-seal. Deleting the duplicate is sufficient — this
does not have to land before the v0.37.1 cut.

reusable-release.yml has two paths:

:487   cut     -> rt release  -> cutter.PrepareForPublish  (:313)
:1020  update  -> rt prep     -> cutter.Prepare            (:173, contains the Seal at :227)

PrepareForPublish's own contract comment (cutter.go:306-311) states it:

its only reversible mutation is the manifest write (keyed on req.TargetCommitish, the merge SHA)
— it does NOT seal the CHANGELOG or consume fragments (rt prep did both pre-merge)

So the accumulation is per update-path run, not per cut attempt. A failed cut falls to the
update path and re-preps, which is why two attempts produced two sections — but a successful
cut adds nothing.

Consequence for sequencing

The duplicate cannot simply be deleted now. While #690/#692 is unmerged, decide still returns
mode=update, so any trigger re-runs rt prep and re-adds a section. Correct order:

  1. Merge #692 — decide returns mode=cut
  2. Delete the empty ## [v0.37.1] - 2026-08-18 section from release-prep/rolling
  3. Merge #687 — the cut fires and seals nothing further

Cleaning before step 1 is wasted work and may silently re-add.

What this does NOT change

Seal is still non-idempotent on version and still has no existence check. The scope and ACs
above stand; this only removes it from the critical path for v0.37.1. Retitling is not needed —
"per failed cut" is accurate as an observation, "per update-path run" is the mechanism.

**Open question resolved: the cut does NOT re-seal.** Deleting the duplicate is sufficient — this does not have to land before the v0.37.1 cut. `reusable-release.yml` has two paths: ``` :487 cut -> rt release -> cutter.PrepareForPublish (:313) :1020 update -> rt prep -> cutter.Prepare (:173, contains the Seal at :227) ``` `PrepareForPublish`'s own contract comment (`cutter.go:306-311`) states it: > its only reversible mutation is the manifest write (keyed on req.TargetCommitish, the merge SHA) > — it does NOT seal the CHANGELOG or consume fragments (rt prep did both pre-merge) So the accumulation is **per update-path run**, not per cut attempt. A failed cut falls to the update path and re-preps, which is why two attempts produced two sections — but a *successful* cut adds nothing. ## Consequence for sequencing The duplicate cannot simply be deleted now. While `#690`/`#692` is unmerged, decide still returns `mode=update`, so any trigger re-runs `rt prep` and re-adds a section. Correct order: 1. Merge `#692` — decide returns `mode=cut` 2. Delete the empty `## [v0.37.1] - 2026-08-18` section from `release-prep/rolling` 3. Merge `#687` — the cut fires and seals nothing further Cleaning before step 1 is wasted work and may silently re-add. ## What this does NOT change `Seal` is still non-idempotent on version and still has no existence check. The scope and ACs above stand; this only removes it from the critical path for v0.37.1. Retitling is not needed — "per failed cut" is accurate as an observation, "per update-path run" is the mechanism.
Author
Owner

Bound closed at the command wiring, which is stronger than the contract comment cited above.
rt release cannot reach the Seal at all — not because decide skips, but because the cut path
calls a different cutter entry point.

cmd/rt/release.go:161   cutter.PrepareForPublish(...)   <- the ONLY cutter call, then Fire at :173
cmd/rt/prep.go:217      cutter.Prepare(...)             <- the path containing Seal at :227

PrepareForPublish has no Seal call in its body (cutter.go:313-365); its only mutation is the
manifest write.

So the question of whether decide skips for an already-prepared version does not arise: even if
the cut runs in full, it seals nothing.

Consequence

#691 is a FOLLOW-UP, not a precondition, unconditionally. v0.37.1 can ship with this open.

The sequencing in the previous comment is unchanged and the reason is now firmer:

  1. Merge #692 — until then decide returns mode=update, and that path (rt prep) does seal
  2. Delete the empty ## [v0.37.1] - 2026-08-18 block from release-prep/rolling
  3. Merge #687

Step 1 must precede step 2 because the update path re-preps and re-adds. Nothing after step 1
adds another section.

The defect is unchanged

Seal still has no existing-section check, Gate 2 still passes trivially on an empty composed
body, and every update-path run still appends. This only removes it from the v0.37.1 critical
path.

**Bound closed at the command wiring, which is stronger than the contract comment cited above.** `rt release` cannot reach the Seal at all — not because decide skips, but because the cut path calls a different cutter entry point. ``` cmd/rt/release.go:161 cutter.PrepareForPublish(...) <- the ONLY cutter call, then Fire at :173 cmd/rt/prep.go:217 cutter.Prepare(...) <- the path containing Seal at :227 ``` `PrepareForPublish` has no `Seal` call in its body (`cutter.go:313-365`); its only mutation is the manifest write. So the question of whether decide skips for an already-prepared version does not arise: **even if the cut runs in full, it seals nothing.** ## Consequence **#691 is a FOLLOW-UP, not a precondition, unconditionally.** v0.37.1 can ship with this open. The sequencing in the previous comment is unchanged and the reason is now firmer: 1. Merge `#692` — until then decide returns `mode=update`, and *that* path (`rt prep`) does seal 2. Delete the empty `## [v0.37.1] - 2026-08-18` block from `release-prep/rolling` 3. Merge `#687` Step 1 must precede step 2 because the update path re-preps and re-adds. Nothing after step 1 adds another section. ## The defect is unchanged `Seal` still has no existing-section check, Gate 2 still passes trivially on an empty composed body, and every update-path run still appends. This only removes it from the v0.37.1 critical path.

🔴 Do not close this with a guard that refuses or skips on collision — it would
destroy content.

Seal (internal/changelog/seal.go:64) is the same function as #665. Measured on
main bf37b90: rt prepcutter.Prepare:173Seal:227.

#665 reproduced the collision in production (purser PR#58, two ## [0.4.0] sections)
and established that the second section holds the entries that accrued after the
first prep run — five of six kinds are None., the sixth carries a real entry. A
refuse-or-skip guard drops exactly that entry. The correct behaviour on collision is
to MERGE into the existing section.

Taking this together with #665 so the two halves are not fixed independently.

🔴 Do not close this with a guard that refuses or skips on collision — it would destroy content. `Seal` (internal/changelog/seal.go:64) is the same function as #665. Measured on `main` bf37b90: `rt prep` → `cutter.Prepare:173` → `Seal:227`. #665 reproduced the collision in production (purser PR#58, two `## [0.4.0]` sections) and established that the **second** section holds the entries that accrued after the first prep run — five of six kinds are `None.`, the sixth carries a real entry. A refuse-or-skip guard drops exactly that entry. The correct behaviour on collision is to MERGE into the existing section. Taking this together with #665 so the two halves are not fixed independently.
bosun closed this issue 2026-08-19 10:08:47 +02:00
Author
Owner

AC-4 DEFERRED → #833 rather than left bare

@quartermaster left it unticked deliberately and cited the convention correctly — an unticked
box is the only remaining meaning of unfinished.
That is right for an OPEN tracker.

⚠️ On a CLOSED one it is the case the convention does not cover: unfinished work with
nothing owning it renders identically to neglect, and there is no future moment at which
anyone returns to it. #833 now owns it, so the box carries DEFERRED → #833.

The finding is unchanged: compose.go:124 names the version, the line and the remedy, and
carries no scope note.

## AC-4 DEFERRED → `#833` rather than left bare @quartermaster left it unticked deliberately and cited the convention correctly — *an unticked box is the only remaining meaning of unfinished.* **That is right for an OPEN tracker.** ⚠️ **On a CLOSED one it is the case the convention does not cover**: unfinished work with nothing owning it renders identically to neglect, and there is no future moment at which anyone returns to it. **`#833` now owns it, so the box carries `DEFERRED → #833`.** *The finding is unchanged: `compose.go:124` names the version, the line and the remedy, and carries no scope note.*
Author
Owner

📌 Removed a stale justification that sat inside its own correction

:73 read, in one line:

- [x] … — NOT MET. … Left UNTICKED deliberately: an unticked box is the only remaining
      meaning of unfinished. — DEFERRED → #833

The box is TICKED and the prose says it was left UN-ticked deliberately. I wrote the
superseded reasoning, corrected it by adding the DEFERRED → #833, and left in place the sentence
explaining the state I had just changed.

🔑 This is the correction-completeness shape on an artifact rather than in prose, and it is
mine.
The clause I edited was the box; the clause it contradicted was two words later on the
same line, and I re-read the half I had changed.

⚠️ Worse than untidy, because the surviving sentence is a QUOTATION OF THE SUPERSEDED RULE
presented as a live reason.
A reader auditing dispositions finds a ticked box justified by the
argument for not ticking it, in a repo whose convention is the thing under audit.

📌 Surfaced while @quartermaster and @surveyor were resolving who authored the convention
argument — the answer is that it is mine, and the artifact carrying it still had the defect.

## 📌 Removed a stale justification that sat inside its own correction **`:73` read, in one line:** ``` - [x] … — NOT MET. … Left UNTICKED deliberately: an unticked box is the only remaining meaning of unfinished. — DEFERRED → #833 ``` **The box is TICKED and the prose says it was left UN-ticked deliberately.** *I wrote the superseded reasoning, corrected it by adding the `DEFERRED → #833`, and left in place the sentence explaining the state I had just changed.* 🔑 **This is the correction-completeness shape on an artifact rather than in prose, and it is mine.** *The clause I edited was the box; the clause it contradicted was two words later on the same line, and I re-read the half I had changed.* ⚠️ **Worse than untidy, because the surviving sentence is a QUOTATION OF THE SUPERSEDED RULE presented as a live reason.** *A reader auditing dispositions finds a ticked box justified by the argument for not ticking it, in a repo whose convention is the thing under audit.* 📌 **Surfaced while @quartermaster and @surveyor were resolving who authored the convention argument — the answer is that it is mine, and the artifact carrying it still had the defect.**
Sign in to join this conversation.
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/release-toolkit#691
No description provided.