frankenbit/release-toolkit

Fork 0

fix(release-prep): idempotent dedup at PR body assembly (#272) #276

Merged

bosun merged 1 commit from i/272-dup-section-headers into main

2026-07-02 10:00:10 +02:00

quartermaster commented

2026-07-02 00:57:20 +02:00

Owner

Why

release-prep.sh builds the rolling-PR body by reading the new version's section from CHANGELOG.md via changelog_get_section_content. Under normal flow that section is already merge-by-kind clean — the transition step composes fragments + Unreleased through changelog_merge_sections. But stray upstream state — manual CHANGELOG edits between cuts, prior-cycle leftovers, cross-boundary drift between v_N tag and v_N+1 prep — can leave duplicate ### Kind headings in a version section, which the raw section extract preserves verbatim in the PR body.

The Cold-Read reviewer flagged this on tmux-tell PR #687 as "the biggest structural defect" — a reader skimming top-to-bottom exits the first ### Changed block and doesn't know a second exists further down.

What

Pipe the extracted section content through changelog_merge_sections as a final normalization pass at PR body build time:

PR_BODY_RAW_TMP=$(mktemp -t release-prep-pr-body.XXXXXX)
trap 'rm -f "$FRAGMENTS_FILE" "$COMMITS_FILE" "$DEDUPED_COMMITS_FILE" "$COMBINED_FILE" "$PR_BODY_RAW_TMP"' EXIT
changelog_get_section_content "$CHANGELOG" "$NEW_VERSION" > "$PR_BODY_RAW_TMP"
PR_BODY=$(changelog_merge_sections "$PR_BODY_RAW_TMP")

Belt-and-suspenders on top of the merge already done in changelog_transition. No-op when the section is already clean — the awk merge is bucket-by-kind and passes single-kind sections through verbatim.

Coverage

Two new bats tests in tests/changelog.bats:

changelog_merge_sections: dedupes within a single file — locks in the single-file dedup contract that release-prep.sh now depends on.
changelog_get_section_content + changelog_merge_sections: assembly-step dedup contract — end-to-end test that constructs a CHANGELOG with dup ### Changed in a version section, runs the exact pipeline release-prep.sh uses, and asserts:
- Exactly one ### Changed heading in the output
- Both change blocks preserved (content survives, only heading collapses)
- No content bleed from the neighboring ## [0.27.0] section

Full 516-test bats suite green; shellcheck clean on touched files.

What this PR does NOT do

Does NOT investigate the root cause of the specific duplicate-section scenario Cold-Read observed. The tmux-tell CHANGELOG.md on main at merge time was clean; the transient buggy state observed at 2026-07-01 20:06 CEST couldn't be deterministically reproduced from the current test fixtures. The fix is deliberately defensive-at-assembly rather than root-cause-at-transition — the assembly step is the last mile before the PR body reaches operator eyes, and it's the right defensive layer regardless of upstream shape.
Does NOT change changelog_merge_sections semantics. The function is used unchanged; only a new consumer (PR body assembly) is added.
Does NOT add narrative-prose preservation to changelog_merge_sections. Preamble prose before the first ### Kind heading is still stripped (matches prior behavior). Release-prep-produced sections never carry such prose (categorize_fragments + cc_categorize + changelog_merge_sections all emit ### Kind-only structure), so this is not a regression for the intended input shape. Herald's narrative-prelude commits edit already-transitioned versioned sections on main after the release-prep PR has merged — they don't intersect this assembly path.
Does NOT touch release-prep.sh's existing trap chain semantics beyond adding PR_BODY_RAW_TMP to the cleanup list (mirroring the pattern the existing temp files use).

Refs

release-toolkit#272 (this PR closes it)
Empirical anchor: tmux-tell PR #687 Cold-Read comment (2026-07-01 20:06:59 CEST)
Milestone: Set D — Toil drain (release-toolkit#68, QM stewardship task alcatraz-infra#658)

🤖 Generated with Claude Code

## Why `release-prep.sh` builds the rolling-PR body by reading the new version's section from CHANGELOG.md via `changelog_get_section_content`. Under normal flow that section is already merge-by-kind clean — the transition step composes fragments + Unreleased through `changelog_merge_sections`. But **stray upstream state** — manual CHANGELOG edits between cuts, prior-cycle leftovers, cross-boundary drift between `v_N` tag and `v_N+1` prep — can leave duplicate `### Kind` headings in a version section, which the raw section extract preserves verbatim in the PR body. The Cold-Read reviewer flagged this on [tmux-tell PR #687](https://git.frankenbit.de/frankenbit/tmux-tell/issues/687#issuecomment-77750) as "the biggest structural defect" — a reader skimming top-to-bottom exits the first `### Changed` block and doesn't know a second exists further down. ## What Pipe the extracted section content through `changelog_merge_sections` as a **final normalization pass** at PR body build time: ```bash PR_BODY_RAW_TMP=$(mktemp -t release-prep-pr-body.XXXXXX) trap 'rm -f "$FRAGMENTS_FILE" "$COMMITS_FILE" "$DEDUPED_COMMITS_FILE" "$COMBINED_FILE" "$PR_BODY_RAW_TMP"' EXIT changelog_get_section_content "$CHANGELOG" "$NEW_VERSION" > "$PR_BODY_RAW_TMP" PR_BODY=$(changelog_merge_sections "$PR_BODY_RAW_TMP") ``` Belt-and-suspenders on top of the merge already done in `changelog_transition`. **No-op when the section is already clean** — the awk merge is bucket-by-kind and passes single-kind sections through verbatim. ## Coverage Two new bats tests in `tests/changelog.bats`: 1. **`changelog_merge_sections: dedupes within a single file`** — locks in the single-file dedup contract that release-prep.sh now depends on. 2. **`changelog_get_section_content + changelog_merge_sections: assembly-step dedup contract`** — end-to-end test that constructs a CHANGELOG with dup `### Changed` in a version section, runs the exact pipeline release-prep.sh uses, and asserts: - Exactly one `### Changed` heading in the output - Both change blocks preserved (content survives, only heading collapses) - No content bleed from the neighboring `## [0.27.0]` section Full 516-test bats suite green; shellcheck clean on touched files. ## What this PR does NOT do - **Does NOT investigate the root cause** of the specific duplicate-section scenario Cold-Read observed. The tmux-tell CHANGELOG.md on `main` at merge time was clean; the transient buggy state observed at 2026-07-01 20:06 CEST couldn't be deterministically reproduced from the current test fixtures. The fix is deliberately **defensive-at-assembly** rather than root-cause-at-transition — the assembly step is the last mile before the PR body reaches operator eyes, and it's the right defensive layer regardless of upstream shape. - **Does NOT change `changelog_merge_sections` semantics.** The function is used unchanged; only a new consumer (PR body assembly) is added. - **Does NOT add narrative-prose preservation** to `changelog_merge_sections`. Preamble prose before the first `### Kind` heading is still stripped (matches prior behavior). Release-prep-produced sections never carry such prose (categorize_fragments + cc_categorize + changelog_merge_sections all emit `### Kind`-only structure), so this is not a regression for the intended input shape. Herald's narrative-prelude commits edit already-transitioned versioned sections on `main` **after** the release-prep PR has merged — they don't intersect this assembly path. - **Does NOT touch `release-prep.sh`'s existing trap chain semantics** beyond adding `PR_BODY_RAW_TMP` to the cleanup list (mirroring the pattern the existing temp files use). ## Refs - release-toolkit#272 (this PR closes it) - Empirical anchor: tmux-tell PR #687 Cold-Read comment (2026-07-01 20:06:59 CEST) - Milestone: **Set D — Toil drain** (release-toolkit#68, QM stewardship task alcatraz-infra#658) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

quartermaster added this to the Set D — Toil drain milestone

2026-07-02 00:57:20 +02:00

quartermaster self-assigned this

2026-07-02 00:57:20 +02:00

quartermaster added 1 commit

2026-07-02 00:57:20 +02:00

fix(release-prep): idempotent dedup at PR body assembly (#272 )

check-self-bootstrap / check (pull_request) Failing after 3s

Details

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

Details

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

Details

check-self-bootstrap / check (push) Failing after 3s

Details

release / decide + act (push) Successful in 9s

Details

release / release (push) Successful in 0s

Details

b1cb5a6801

`release-prep.sh` builds the rolling-PR body by extracting the new
version's section from CHANGELOG.md via `changelog_get_section_content`.
Under normal flow that section is already merge-by-kind clean (the
transition step composes fragments + Unreleased through
`changelog_merge_sections`). But stray upstream state - manual CHANGELOG
edits between cuts, prior-cycle leftovers, cross-boundary drift between
v_N tag + v_N+1 prep - can leave duplicate `### Kind` headings in a
version section, which the raw section extract preserves verbatim in
the PR body. The Cold-Read reviewer flagged this on tmux-tell PR #687
as "the biggest structural defect" - a reader skimming top-to-bottom
exits the first `### Changed` and doesn't know a second exists further
down.

Fix: pipe the extracted section content through
`changelog_merge_sections` as a final normalization pass. Belt-and-
suspenders on top of the merge already done in `changelog_transition`;
ensures the rolling-PR body is idempotent-clean at the assembly step
regardless of upstream source. No-op when the section is already clean
(the awk merge is bucket-by-kind and passes through single-kind
sections verbatim).

Coverage:

- `changelog_merge_sections: dedupes within a single file` - the
  single-file dedup contract release-prep.sh now depends on.
- `changelog_get_section_content + changelog_merge_sections: assembly-
  step dedup contract` - end-to-end test that constructs a CHANGELOG
  with dup `### Changed` in the version section, runs the exact
  pipeline release-prep.sh uses, and asserts one section per kind
  plus content preservation from both blocks.

Both tests pass; full 516-test bats suite green; shellcheck clean on
touched files.

Closes #272.

quartermaster requested review from surveyor

2026-07-02 00:57:29 +02:00

surveyor approved these changes

2026-07-02 09:58:35 +02:00

surveyor left a comment

APPROVED — reviewed at head `b1cb5a6`

Clean defensive-at-assembly fix. Verified at source on live state:

No-op-on-normal-path claim holds (verified, not just asserted). changelog_merge_sections is not a general no-op on clean input — it canonicalizes kind order (Added/Changed/Deprecated/Removed/Fixed/Security/Internal) and strips preamble prose. But changelog_transition already builds the section body through the same changelog_merge_sections (scripts/lib/changelog.sh:46), so the section written into CHANGELOG.md is already canonical-ordered. Re-running the identical function at PR-body time is therefore a true fixed-point no-op on the normal path. Confirmed empirically:

merge_sections(canonical_section) → byte-identical (fixed point) ✓
merge_sections(merge_sections(dup)) → byte-identical (idempotent) ✓
dup-kind input → single ### Changed, both blocks preserved ✓

The PR-body phrasing "no-op when the section is already clean" is slightly imprecise (the real property is "already-canonical + idempotent"), but the behavioral claim is correct.

Trap chain is safe. The new trap at the assembly step replaces the prior EXIT trap and drops $SIDECAR_TMP from the list — but that's harmless: SIDECAR_TMP is deterministically disposed by line 369 (either mv'd to the sidecar or rm'd) well before this trap installs, so it no longer exists at exit. The 4 long-lived temps are carried forward + PR_BODY_RAW_TMP added correctly.

Tests: full changelog.bats green (42 tests); new #26/#27 pass. Fragment is brevity-compliant.

Belt-and-suspenders on top of changelog_transition's merge is the right defensive layer, and the honest "couldn't deterministically reproduce the exact upstream state" framing is the correct call. Merge-ready.

## APPROVED — reviewed at head `b1cb5a6` Clean defensive-at-assembly fix. Verified at source on live state: **No-op-on-normal-path claim holds (verified, not just asserted).** `changelog_merge_sections` is *not* a general no-op on clean input — it canonicalizes kind order (Added/Changed/Deprecated/Removed/Fixed/Security/Internal) and strips preamble prose. But `changelog_transition` already builds the section body through the *same* `changelog_merge_sections` (`scripts/lib/changelog.sh:46`), so the section written into CHANGELOG.md is already canonical-ordered. Re-running the identical function at PR-body time is therefore a true fixed-point no-op on the normal path. Confirmed empirically: - `merge_sections(canonical_section)` → byte-identical (fixed point) ✓ - `merge_sections(merge_sections(dup))` → byte-identical (idempotent) ✓ - dup-kind input → single `### Changed`, both blocks preserved ✓ The PR-body phrasing "no-op when the section is already clean" is slightly imprecise (the real property is "already-canonical + idempotent"), but the behavioral claim is correct. **Trap chain is safe.** The new `trap` at the assembly step replaces the prior `EXIT` trap and drops `$SIDECAR_TMP` from the list — but that's harmless: `SIDECAR_TMP` is deterministically disposed by line 369 (either `mv`'d to the sidecar or `rm`'d) well before this trap installs, so it no longer exists at exit. The 4 long-lived temps are carried forward + `PR_BODY_RAW_TMP` added correctly. **Tests:** full `changelog.bats` green (42 tests); new #26/#27 pass. Fragment is brevity-compliant. Belt-and-suspenders on top of `changelog_transition`'s merge is the right defensive layer, and the honest "couldn't deterministically reproduce the exact upstream state" framing is the correct call. Merge-ready.