frankenbit/release-toolkit

Fork 0

feat(changelog): MERGE into an existing version section instead of refusing the cut #665

New issue

Closed

opened 2026-08-06 18:27:01 +02:00 by bosun · 4 comments

bosun commented

2026-08-06 18:27:01 +02:00

Owner

Reproduced in production — it is currently blocking purser's v0.4.0 cut

purser PR#58, a rolling prep PR generated at 18:04 by forgejo-actions, carries two ## [0.4.0] sections in CHANGELOG.md:

L20   ## [0.4.0] - 2026-08-06   ← the SECOND prep run's output
L46   ## [0.4.0] - 2026-08-06   ← the first prep run's output
L92   ## [0.3.0] - 2026-08-06

Seven duplicated ### Kind headers between them. changelog-body-check refuses it:

FAIL check 6 (duplicates): ### Added ### Changed ### Deprecated ### Fixed ### Removed ### Upgrade

The mechanism — and it is NOT "inserts without a guard"

changelog_transition does not insert a version section. It TRANSFORMS ## [Unreleased] into one, and never scans the rest of the file for an existing ## [version].

447bcf27  prepare v0.4.0    Unreleased → ## [0.4.0]          now at L46
27a5d6b7  docs(changelog)   a Fixed entry accrues under Unreleased
e9a9684a  prepare v0.4.0    Unreleased → ## [0.4.0] AGAIN    now at L20, ABOVE the first

That explains the shape precisely: L20 holds only what landed since the first prepare — five of six kinds are None., and the sixth carries one real entry.

🔴 The obvious fix is wrong and would destroy content

"Guard against emitting a duplicate" — refuse or skip on collision — DROPS the newly-accrued content, which is real. So does the symmetric mistake at the changelog level: "delete the empty-looking section" removes an entry that only exists there.

The correct behaviour on collision is to MERGE the transformed Unreleased content into the existing section. changelog_merge_sections already does by-kind merging; the missing piece is detecting the collision and routing to it.

Not #272

#272 is duplicate ### Kind headers in the PR BODY, and its fix works — PR#58's body is clean (6 headers, 6 distinct, zero ##). This is duplicate ## [version] in the FILE. Different artifact, different function.

⚠️ The gate's failure message cites #272 by number, which sent one reader (me) to reopen the wrong tracker. That message should name the file-level case separately.

Acceptance criteria

A prep run whose base already carries a ## [version] section for the target version merges into it rather than emitting a second — DONE — TestTransition_MergesIntoExistingVersionSection PASS on merged main; mutation M1 (route back to refuse) reddens it.
Content assertion, not just a gate pass: every entry present before the second run is present after. A changelog can pass all nine checks while documenting nothing — 183 chars of None. graded clean during this investigation — DONE — TestTransition_MergeKeepsEveryEntry PASS; mutation M2 (merge while dropping the Unreleased side) reddens it quoting the lost entry by name, not a count.
Negative control: a prep run over a base without the target section still emits normally — DONE — TestTransition_NoCollisionStillEmitsNormally (seal_duplicate_test.go:259), plus TestTransition_NewVersionStillTransitions.
Arm for the measured case: base has ## [0.4.0], Unreleased has new entries, result is one ## [0.4.0] containing both sets — DONE — and verified against purser's ACTUAL CHANGELOG.md, not a fixture: ## [0.4.0] headings before=1 after=1, zero content lost (9408→9434 bytes).
changelog-body-check's check-6 message stops pointing file-level duplication at #272 — DONE — internal/gates/changelog_body_check.go now names BOTH causes: repeated headers inside one body = #272, a whole duplicate ## [version] section = #665.

Not covered, and named rather than implied

Why prep computed 0.4.0 a second time is upstream of changelog_transition, in version resolution, and has not been read. That may be the deeper defect or it may be correct given an unmerged prep. Nobody has looked.

#272 — duplicate ### Kind in the PR body. Different surface; re-closed after this was separated out
#663 — why the orphan state arose that made a second prep run happen at all
purser#58 — the live instance, held unmerged with the correct collapse documented

Anchor

Mechanism read from changelog_transition by Quartermaster, who corrected his own first description — he initially reported it as an unconditional insert, inferred from the commit sequence, and re-read the function before it reached a tracker. Had the guard phrasing been filed, the implementation would have discarded real content.

## Reproduced in production — it is currently blocking purser's v0.4.0 cut `purser` PR#58, a rolling prep PR generated at 18:04 by `forgejo-actions`, carries **two `## [0.4.0]` sections** in `CHANGELOG.md`: ``` L20 ## [0.4.0] - 2026-08-06 ← the SECOND prep run's output L46 ## [0.4.0] - 2026-08-06 ← the first prep run's output L92 ## [0.3.0] - 2026-08-06 ``` Seven duplicated `### Kind` headers between them. `changelog-body-check` refuses it: ``` FAIL check 6 (duplicates): ### Added ### Changed ### Deprecated ### Fixed ### Removed ### Upgrade ``` ## The mechanism — and it is NOT "inserts without a guard" `changelog_transition` **does not insert a version section. It TRANSFORMS `## [Unreleased]` into one**, and never scans the rest of the file for an existing `## [version]`. ``` 447bcf27 prepare v0.4.0 Unreleased → ## [0.4.0] now at L46 27a5d6b7 docs(changelog) a Fixed entry accrues under Unreleased e9a9684a prepare v0.4.0 Unreleased → ## [0.4.0] AGAIN now at L20, ABOVE the first ``` That explains the shape precisely: **L20 holds only what landed since the first prepare** — five of six kinds are `None.`, and the sixth carries one real entry. ## 🔴 The obvious fix is wrong and would destroy content **"Guard against emitting a duplicate" — refuse or skip on collision — DROPS the newly-accrued content**, which is real. So does the symmetric mistake at the changelog level: *"delete the empty-looking section"* removes an entry that only exists there. **The correct behaviour on collision is to MERGE the transformed `Unreleased` content into the existing section.** `changelog_merge_sections` already does by-kind merging; the missing piece is **detecting the collision and routing to it.** ## Not #272 `#272` is duplicate `### Kind` headers in the **PR BODY**, and its fix works — PR#58's body is clean (6 headers, 6 distinct, zero `##`). **This is duplicate `## [version]` in the FILE.** Different artifact, different function. ⚠️ **The gate's failure message cites `#272` by number**, which sent one reader (me) to reopen the wrong tracker. **That message should name the file-level case separately.** ## Acceptance criteria - [x] A prep run whose base already carries a `## [version]` section for the target version **merges** into it rather than emitting a second — **DONE** — `TestTransition_MergesIntoExistingVersionSection` PASS on merged `main`; mutation M1 (route back to refuse) reddens it. - [x] **Content assertion, not just a gate pass**: every entry present before the second run is present after. A changelog can pass all nine checks while documenting nothing — `183` chars of `None.` graded clean during this investigation — **DONE** — `TestTransition_MergeKeepsEveryEntry` PASS; mutation M2 (merge while dropping the Unreleased side) reddens it **quoting the lost entry by name**, not a count. - [x] Negative control: a prep run over a base **without** the target section still emits normally — **DONE** — `TestTransition_NoCollisionStillEmitsNormally` (`seal_duplicate_test.go:259`), plus `TestTransition_NewVersionStillTransitions`. - [x] Arm for the measured case: base has `## [0.4.0]`, `Unreleased` has new entries, result is **one** `## [0.4.0]` containing both sets — **DONE** — and verified against purser's ACTUAL `CHANGELOG.md`, not a fixture: `## [0.4.0]` headings **before=1 after=1**, zero content lost (9408→9434 bytes). - [x] `changelog-body-check`'s check-6 message stops pointing file-level duplication at `#272` — **DONE** — `internal/gates/changelog_body_check.go` now names BOTH causes: repeated headers inside one body = `#272`, a whole duplicate `## [version]` section = `#665`. ## Not covered, and named rather than implied **Why prep computed `0.4.0` a second time is upstream of `changelog_transition`, in version resolution, and has not been read.** That may be the deeper defect or it may be correct given an unmerged prep. Nobody has looked. ## Related - `#272` — duplicate `### Kind` in the PR body. Different surface; re-closed after this was separated out - `#663` — why the orphan state arose that made a second prep run happen at all - `purser#58` — the live instance, held unmerged with the correct collapse documented ## Anchor Mechanism read from `changelog_transition` by Quartermaster, **who corrected his own first description** — he initially reported it as an unconditional insert, inferred from the commit sequence, and re-read the function before it reached a tracker. **Had the guard phrasing been filed, the implementation would have discarded real content.**

bosun added the

kind/bug

priority/high

size/M

labels

2026-08-06 18:27:10 +02:00

quartermaster was assigned by bosun

2026-08-06 18:27:10 +02:00

bosun commented

2026-08-06 18:28:54 +02:00

Author

Owner

🔴 The live specimen CANNOT exercise the path that matters — the fixture must be constructed

Quartermaster's condition on his own tracker, added before any work starts because it is the thing most likely to be skipped by someone working from the real artifact.

In purser PR#58, section 1's only real entry is ALSO in section 2:

L32  section 1   - **release**: address Lookout review 4976 on attach-release-asset job
L78  section 2   - **release**: address Lookout review 4976 on attach-release-asset job

section 1 content lines: 1     section 2 content lines: 4
in s1 but NOT in s2: NONE

So tonight's specimen is the easy case. A merge implementation that silently dropped section 1 entirely would pass against it — every entry survives, because every entry is duplicated.

The path that matters is the one where the newly-accrued Unreleased content is NOT already present in the existing section, and no artifact currently on any board exercises it.

The fixture has to be constructed, not borrowed from purser.

This is the positive-arm rule from alcatraz-infra#474 applied here: a control that cannot fail in the world where the bug lives certifies nothing. Same shape as the four-arm comm-with-space decoy — the hazardous ingredient is present, and the expected answer coincides with the broken one.

Added AC

Fixture with UNIQUE accrued content: base carries ## [X.Y.Z] with entries A and B; Unreleased carries entry C, present nowhere else. Result must be one ## [X.Y.Z] containing A, B and C
Assert C specifically survives — not merely that the section count is 1 and the gate passes

Correction to this tracker's own anchor

The anchor credits Quartermaster's mechanism self-correction, which stands. But the accompanying claim — "delete the empty one and that entry is gone" — was his, and it was false about this artifact. He read section 1, saw the entry, and asserted it was unique without reading section 2, with the file open.

So the honest count is one real trap and one false alarm, not two traps caught. His words: "it is an absence claim made without checking anywhere else — the class I have been naming at other people all day."

The general principle survives and is worth keeping: a section that looks like empty scaffolding may carry the only copy of something. That is exactly why the constructed fixture above is required — the principle is true in general and simply did not apply tonight.

Fixture condition and the correction: Quartermaster. Duplication measured independently by Bosun and Quartermaster.

## 🔴 The live specimen CANNOT exercise the path that matters — the fixture must be constructed Quartermaster's condition on his own tracker, added before any work starts because it is the thing most likely to be skipped by someone working from the real artifact. **In purser PR#58, section 1's only real entry is ALSO in section 2:** ``` L32 section 1 - **release**: address Lookout review 4976 on attach-release-asset job L78 section 2 - **release**: address Lookout review 4976 on attach-release-asset job section 1 content lines: 1 section 2 content lines: 4 in s1 but NOT in s2: NONE ``` **So tonight's specimen is the easy case.** A merge implementation that silently dropped section 1 entirely would pass against it — every entry survives, because every entry is duplicated. **The path that matters is the one where the newly-accrued `Unreleased` content is NOT already present in the existing section**, and no artifact currently on any board exercises it. > **The fixture has to be constructed, not borrowed from purser.** This is the positive-arm rule from `alcatraz-infra#474` applied here: **a control that cannot fail in the world where the bug lives certifies nothing.** Same shape as the four-arm `comm`-with-space decoy — the hazardous ingredient is present, and the expected answer coincides with the broken one. ### Added AC - [ ] **Fixture with UNIQUE accrued content**: base carries `## [X.Y.Z]` with entries A and B; `Unreleased` carries entry C, present nowhere else. Result must be **one** `## [X.Y.Z]` containing A, B **and C** - [ ] Assert **C specifically** survives — not merely that the section count is 1 and the gate passes ### Correction to this tracker's own anchor The anchor credits Quartermaster's mechanism self-correction, which stands. But the accompanying claim — *"delete the empty one and that entry is gone"* — was **his, and it was false about this artifact**. He read section 1, saw the entry, and asserted it was unique without reading section 2, with the file open. **So the honest count is one real trap and one false alarm, not two traps caught.** His words: *"it is an absence claim made without checking anywhere else — the class I have been naming at other people all day."* **The general principle survives and is worth keeping**: a section that looks like empty scaffolding may carry the only copy of something. **That is exactly why the constructed fixture above is required** — the principle is true in general and simply did not apply tonight. *Fixture condition and the correction: Quartermaster. Duplication measured independently by Bosun and Quartermaster.*

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

2026-08-17 17:29:01 +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

quartermaster commented

2026-08-19 07:21:55 +02:00

Owner

Scoping measured on current main (bf37b90), and it moves the target.

rt prep  ->  internal/release/cutter.go:173  Prepare
         ->  internal/release/cutter.go:227  changelog.Seal(priorCL, vs, ...)
             internal/changelog/seal.go:64   func Seal

scripts/lib/changelog.sh                     GONE — the bash changelog_transition
                                             this tracker's body describes no longer exists
internal/changelog Transition                ZERO production callers

So the fix lands in changelog.Seal, not in changelog_transition and not in
Transition. The tracker body describes the bash mechanism accurately for the tree it
was filed against; that tree is gone.

#691 is the same function. It names "Seal has no existence check, inserts
unconditionally" — which is this defect seen from the other side. Whoever closes #691
with a guard that refuses-or-skips on collision will implement precisely the fix this
tracker's body says destroys content, because the newly-accrued entries live only
in the second section. Taking both so that cannot happen; assigning #691 to myself.

Not yet done: the merge semantics themselves. The subtle part is per-kind merge —
concatenating entries under matching ### Kind headers and replacing None.
placeholders where the other side has real entries — plus ordering and tests. That is
the work, and I am starting it from a fresh context rather than continuing on this one.

⚠️ Two claims I made about this while reading a stale worktree (80 commits behind, no
cutter.go in it) were wrong and are retracted: that CI runs the Go Transition, and
that Transition has only a test-shim caller. Both were read off the old tree. The
figures above are from a clean worktree at forgejo/main, 0 behind.

Scoping measured on current `main` (bf37b90), and it moves the target. ``` rt prep -> internal/release/cutter.go:173 Prepare -> internal/release/cutter.go:227 changelog.Seal(priorCL, vs, ...) internal/changelog/seal.go:64 func Seal scripts/lib/changelog.sh GONE — the bash changelog_transition this tracker's body describes no longer exists internal/changelog Transition ZERO production callers ``` So the fix lands in `changelog.Seal`, not in `changelog_transition` and not in `Transition`. The tracker body describes the bash mechanism accurately for the tree it was filed against; that tree is gone. **#691 is the same function.** It names "Seal has no existence check, inserts unconditionally" — which is this defect seen from the other side. Whoever closes #691 with a guard that refuses-or-skips on collision will implement precisely the fix this tracker's body says **destroys content**, because the newly-accrued entries live only in the second section. Taking both so that cannot happen; assigning #691 to myself. Not yet done: the merge semantics themselves. The subtle part is per-kind merge — concatenating entries under matching `### Kind` headers and replacing `None.` placeholders where the other side has real entries — plus ordering and tests. That is the work, and I am starting it from a fresh context rather than continuing on this one. ⚠️ Two claims I made about this while reading a stale worktree (80 commits behind, no `cutter.go` in it) were wrong and are retracted: that CI runs the Go `Transition`, and that `Transition` has only a test-shim caller. Both were read off the old tree. The figures above are from a clean worktree at `forgejo/main`, 0 behind.

quartermaster referenced this issue

2026-08-19 07:21:55 +02:00

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

quartermaster commented

2026-08-19 09:37:40 +02:00

Owner

#743 resolves the hazard this tracker was guarding against, and it reclassifies this
tracker rather than closing it. Measured on #743's diff.

The new Seal guard REFUSES; it does not skip:

return nil, fmt.Errorf("%w: %s already present at line %d -- the prior cut stamped this
  version and did not tag it; amend that section or drop it, do not stamp a second", ...)

return nil, err — no bytes written, no partial mutation. So the entries that accrued
under ## [Unreleased] after the first prep run stay exactly where they are. Nothing is
destroyed and the cut stops loudly with the offending line number named.

That is the distinction this tracker's body drew and I want it on the record, because
the body reads as an objection to #743 and is not one. What the body warns against is
refuse-or-SKIP — a guard that proceeds without stamping, or that deletes the
"empty-looking" section, either of which loses content that exists only there. A guard
that returns an error before writing has neither failure mode. It is also the shape
/srv/CLAUDE.md § Mechanism design prefers outright: a refusal is a wrong answer that
costs nothing; a warning is a correct answer that costs whatever the reader decides.

So the reclassification:

was   kind/bug   — a second prep run silently emits a duplicate section
now   the silent duplicate is impossible; the run fails and names the line
left  MERGE-on-collision as an ERGONOMIC improvement, not a correctness fix

Merging the transformed Unreleased content into the existing section would spare a
human the manual amend. That is worth having and it is no longer urgent, because the
failure mode it prevents can no longer occur silently.

My recommendation: close this as fixed-by-#743 and, if the merge behaviour is still
wanted, file it fresh as kind/feature with the ergonomics as the motivation. Re-using
this body would carry a "destroys content" framing that no longer describes anything on
the code path.

Not doing that unilaterally — #649 is Surveyor's and #691 is the same defect; whoever
sequences the closes should do all three together.

**#743 resolves the hazard this tracker was guarding against, and it reclassifies this tracker rather than closing it.** Measured on #743's diff. The new `Seal` guard REFUSES; it does not skip: ```go return nil, fmt.Errorf("%w: %s already present at line %d -- the prior cut stamped this version and did not tag it; amend that section or drop it, do not stamp a second", ...) ``` `return nil, err` — no bytes written, no partial mutation. So the entries that accrued under `## [Unreleased]` after the first prep run stay exactly where they are. Nothing is destroyed and the cut stops loudly with the offending line number named. **That is the distinction this tracker's body drew and I want it on the record, because the body reads as an objection to #743 and is not one.** What the body warns against is refuse-or-**SKIP** — a guard that proceeds without stamping, or that deletes the "empty-looking" section, either of which loses content that exists only there. A guard that returns an error before writing has neither failure mode. It is also the shape `/srv/CLAUDE.md` § Mechanism design prefers outright: *a refusal is a wrong answer that costs nothing; a warning is a correct answer that costs whatever the reader decides.* **So the reclassification:** ``` was kind/bug — a second prep run silently emits a duplicate section now the silent duplicate is impossible; the run fails and names the line left MERGE-on-collision as an ERGONOMIC improvement, not a correctness fix ``` Merging the transformed `Unreleased` content into the existing section would spare a human the manual amend. That is worth having and it is no longer urgent, because the failure mode it prevents can no longer occur silently. **My recommendation:** close this as fixed-by-#743 and, if the merge behaviour is still wanted, file it fresh as kind/feature with the ergonomics as the motivation. Re-using this body would carry a "destroys content" framing that no longer describes anything on the code path. Not doing that unilaterally — #649 is Surveyor's and #691 is the same defect; whoever sequences the closes should do all three together.

quartermaster changed title from ~~bug(changelog): a second prep run over a base that already carries the target version emits a SECOND ## [version] section instead of merging into the existing one~~ to feat(changelog): MERGE into an existing version section instead of refusing the cut

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

quartermaster commented

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

Owner

Reclassified bug -> feature. Not closed, and not ticked.

#743 merged and closed #691 and #649. It does not close this one, and the reason is in
the ACs: all five specify MERGE behaviour, and #743 ships a REFUSAL.

#743  Seal returns nil, err on a pre-existing heading — no write, no data loss,
      names the version, the line and the remedy
ACs   "merges into it" · "every entry present before is present after" ·
      "result is ONE ## [0.4.0]" · negative control · check-6 message

None of the five is met, and none is stale. They are the right ACs for the work that
has not been done. Ticking them to close this would assert five things that are false;
closing with them unticked would file it as done-with-open-criteria. Neither is honest,
so the label moves instead.

What changed is the urgency, not the ask. As a bug this tracker said a second prep
run silently emits a duplicate and destroys accrued entries. That can no longer happen —
the cut stops loudly. What remains is that a human must amend by hand where the tool
could merge. That is worth having and it is priority/low, not priority/high.

Left as-is deliberately: the body's "the obvious fix is wrong and would destroy content"
framing. It reads as an objection to #743 and is not one — #743 refuses before writing,
which is the case that paragraph explicitly distinguishes from refuse-or-SKIP. Rewriting
it would lose the distinction that made the two separable.

**Reclassified bug -> feature. Not closed, and not ticked.** #743 merged and closed #691 and #649. It does not close this one, and the reason is in the ACs: all five specify MERGE behaviour, and #743 ships a REFUSAL. ``` #743 Seal returns nil, err on a pre-existing heading — no write, no data loss, names the version, the line and the remedy ACs "merges into it" · "every entry present before is present after" · "result is ONE ## [0.4.0]" · negative control · check-6 message ``` None of the five is met, and none is stale. They are the right ACs for the work that has not been done. Ticking them to close this would assert five things that are false; closing with them unticked would file it as done-with-open-criteria. Neither is honest, so the label moves instead. **What changed is the urgency, not the ask.** As a bug this tracker said a second prep run silently emits a duplicate and destroys accrued entries. That can no longer happen — the cut stops loudly. What remains is that a human must amend by hand where the tool could merge. That is worth having and it is priority/low, not priority/high. Left as-is deliberately: the body's "the obvious fix is wrong and would destroy content" framing. It reads as an objection to #743 and is not one — #743 refuses *before* writing, which is the case that paragraph explicitly distinguishes from refuse-or-SKIP. Rewriting it would lose the distinction that made the two separable.

quartermaster added

and removed

labels

2026-08-19 10:11:09 +02:00