From 1c15fa8e0871df54de5ebeb97332e6d2edf9e133 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Mon, 17 Aug 2026 07:25:30 +0000 Subject: [PATCH 1/3] collate_changelog: skip duplicate version section on rerun after partial failure If the version header is already present in CHANGELOG.md, consume any leftover fragments and return 0 instead of inserting the section again. This makes reruns after an interrupted first run idempotent. --- ...tsk-arsogp-collate-changelog-idempotent.md | 2 ++ scripts/collate_changelog.py | 9 ++++++ tests/test_collate_changelog.py | 32 +++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 changelog.d/tsk-arsogp-collate-changelog-idempotent.md diff --git a/changelog.d/tsk-arsogp-collate-changelog-idempotent.md b/changelog.d/tsk-arsogp-collate-changelog-idempotent.md new file mode 100644 index 000000000..a7743248b --- /dev/null +++ b/changelog.d/tsk-arsogp-collate-changelog-idempotent.md @@ -0,0 +1,2 @@ +### Fixed +- `scripts/collate_changelog.py` is now idempotent across partial failures: if a run dies between writing the new version section and unlinking consumed fragments, a rerun detects the existing `## []` header and skips the duplicate insert while still consuming any leftover fragments. diff --git a/scripts/collate_changelog.py b/scripts/collate_changelog.py index a7da26837..a705e9bc9 100644 --- a/scripts/collate_changelog.py +++ b/scripts/collate_changelog.py @@ -106,6 +106,15 @@ def main(argv: list[str]) -> int: if UNRELEASED not in text: print(f"collate-changelog: {CHANGELOG.name} has no '{UNRELEASED}' anchor", file=sys.stderr) return 1 + + version_header = f"## [{args.version}]" + if version_header in text: + print(f"collate-changelog: {args.version} section already present in {CHANGELOG.name}, consuming leftover fragments", file=sys.stderr) + for path in consumed: + path.unlink() + print(f"collate-changelog: consumed {len(consumed)} leftover fragment(s) for {args.version}") + return 0 + # Insert directly BELOW [Unreleased] so Unreleased stays empty and on top, # which is what the release train expects on the next cycle. anchor = UNRELEASED + "\n" diff --git a/tests/test_collate_changelog.py b/tests/test_collate_changelog.py index 1c0475bfb..bf4ed63e6 100644 --- a/tests/test_collate_changelog.py +++ b/tests/test_collate_changelog.py @@ -114,3 +114,35 @@ def test_missing_unreleased_anchor_fails_loudly_and_keeps_fragments(repo: Path): assert mod.main(["1.0.0-beta.47"]) == 1 # The fragment survives a failed run so nothing is lost. assert (repo / "changelog.d" / "2295-z.md").exists() + + +def test_rerun_after_partial_unlink_failure_is_idempotent(repo: Path, monkeypatch): + mod = _load(repo) + (repo / "changelog.d" / "2291-notes.md").write_text("- Notes area (#2291).\n", encoding="utf-8") + + fail_once = True + import pathlib + + _real_unlink = pathlib.Path.unlink + + def fake_unlink(self): + nonlocal fail_once + if fail_once: + fail_once = False + raise OSError("simulated unlink failure") + _real_unlink(self) + + monkeypatch.setattr(pathlib.Path, "unlink", fake_unlink) + + # First run: writes the section, then fails on unlink (exception propagates). + with pytest.raises(OSError, match="simulated unlink failure"): + mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) + + # Second run: fragment is still present, so without idempotency the section + # would be inserted a second time. + assert mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) == 0 + + text = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + assert text.count("## [1.0.0-beta.47]") == 1 + assert "- Notes area (#2291)." in text + assert not (repo / "changelog.d" / "2291-notes.md").exists() From 099bc9d771b008ccc837c9e71220d1ad41cb799f Mon Sep 17 00:00:00 2001 From: jaylfc Date: Mon, 17 Aug 2026 09:22:16 +0000 Subject: [PATCH 2/3] collate_changelog: rerun consumes only folded fragments, refuses loudly on unfolded ones A fragment that lands between the failed run and the rerun is folded nowhere; unlinking it silently loses its release note. The rerun now checks each leftover fragment's lines against CHANGELOG.md, consumes the folded ones, keeps the unfolded ones and exits non-zero naming them. Documented rerun semantics in docs/changelog-fragments.md. --- ...tsk-arsogp-collate-changelog-idempotent.md | 2 +- docs/changelog-fragments.md | 7 +++ scripts/collate_changelog.py | 19 ++++++-- tests/test_collate_changelog.py | 45 +++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/changelog.d/tsk-arsogp-collate-changelog-idempotent.md b/changelog.d/tsk-arsogp-collate-changelog-idempotent.md index a7743248b..5cfbe2244 100644 --- a/changelog.d/tsk-arsogp-collate-changelog-idempotent.md +++ b/changelog.d/tsk-arsogp-collate-changelog-idempotent.md @@ -1,2 +1,2 @@ ### Fixed -- `scripts/collate_changelog.py` is now idempotent across partial failures: if a run dies between writing the new version section and unlinking consumed fragments, a rerun detects the existing `## []` header and skips the duplicate insert while still consuming any leftover fragments. +- `scripts/collate_changelog.py` is now idempotent across partial failures: if a run dies between writing the new version section and unlinking consumed fragments, a rerun detects the existing `## []` header and skips the duplicate insert. Only leftover fragments whose content already reached `CHANGELOG.md` are consumed; a fragment that landed after the failed run is kept and the rerun exits non-zero naming it, instead of silently deleting a release note that was never folded. diff --git a/docs/changelog-fragments.md b/docs/changelog-fragments.md index e38d58347..7e3390196 100644 --- a/docs/changelog-fragments.md +++ b/docs/changelog-fragments.md @@ -46,3 +46,10 @@ which folds every fragment into a new `## [] - ` section beneath `[Unreleased]`, groups them by section in Keep-a-Changelog order, and deletes the fragments in the same commit. `--dry-run` prints the section without touching anything. + +Reruns are safe: if the `## []` section already exists (a previous run +died between writing the section and deleting the fragments), the collator does +not insert a duplicate. It consumes only leftover fragments whose content is +already present in `CHANGELOG.md`; a fragment that landed after the failed run +is folded nowhere, so it is kept on disk and the rerun exits non-zero naming +it — fold it by rerunning with the correct (next) target version. diff --git a/scripts/collate_changelog.py b/scripts/collate_changelog.py index a705e9bc9..e5cc87f60 100644 --- a/scripts/collate_changelog.py +++ b/scripts/collate_changelog.py @@ -109,10 +109,23 @@ def main(argv: list[str]) -> int: version_header = f"## [{args.version}]" if version_header in text: - print(f"collate-changelog: {args.version} section already present in {CHANGELOG.name}, consuming leftover fragments", file=sys.stderr) + # Rerun after a partial failure: only consume fragments whose content + # already made it into the changelog. A fragment that landed AFTER the + # failed run is folded nowhere -- unlinking it would silently lose its + # release note, so keep it and refuse loudly instead. + print(f"collate-changelog: {args.version} section already present in {CHANGELOG.name}, consuming folded leftover fragments", file=sys.stderr) + unfolded: list[Path] = [] for path in consumed: - path.unlink() - print(f"collate-changelog: consumed {len(consumed)} leftover fragment(s) for {args.version}") + lines = [ln for section_lines in parse_fragment(path).values() for ln in section_lines] + if all(ln in text for ln in lines): + path.unlink() + else: + unfolded.append(path) + print(f"collate-changelog: consumed {len(consumed) - len(unfolded)} leftover fragment(s) for {args.version}") + if unfolded: + for path in unfolded: + print(f"collate-changelog: {path.name} is not folded into {CHANGELOG.name} -- kept; rerun with the correct target version", file=sys.stderr) + return 1 return 0 # Insert directly BELOW [Unreleased] so Unreleased stays empty and on top, diff --git a/tests/test_collate_changelog.py b/tests/test_collate_changelog.py index bf4ed63e6..84b3c530f 100644 --- a/tests/test_collate_changelog.py +++ b/tests/test_collate_changelog.py @@ -146,3 +146,48 @@ def fake_unlink(self): assert text.count("## [1.0.0-beta.47]") == 1 assert "- Notes area (#2291)." in text assert not (repo / "changelog.d" / "2291-notes.md").exists() + + +def test_rerun_keeps_fragment_that_landed_after_the_partial_failure(repo: Path, monkeypatch): + """A fragment merged between the failed run and the rerun was never folded. + + The rerun must not silently unlink it: its content exists nowhere in + CHANGELOG.md, so deleting it loses the release note. Stale (already + folded) leftovers are still consumed; the unfolded one is kept and the + rerun refuses loudly so the operator folds it under the right version. + """ + mod = _load(repo) + (repo / "changelog.d" / "2291-notes.md").write_text("- Notes area (#2291).\n", encoding="utf-8") + + fail_once = True + import pathlib + + _real_unlink = pathlib.Path.unlink + + def fake_unlink(self): + nonlocal fail_once + if fail_once: + fail_once = False + raise OSError("simulated unlink failure") + _real_unlink(self) + + monkeypatch.setattr(pathlib.Path, "unlink", fake_unlink) + + with pytest.raises(OSError, match="simulated unlink failure"): + mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) + + # A new PR merges its fragment between the failed run and the rerun. + (repo / "changelog.d" / "2299-new.md").write_text("- Brand new feature (#2299).\n", encoding="utf-8") + + rc = mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) + text = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + + # The stale leftover was already folded by the first run: consumed. + assert not (repo / "changelog.d" / "2291-notes.md").exists() + # The unfolded fragment survives, its content is not lost and not + # half-inserted anywhere. + assert (repo / "changelog.d" / "2299-new.md").exists() + assert "- Brand new feature (#2299)." not in text + assert text.count("## [1.0.0-beta.47]") == 1 + # And the rerun says NO loudly instead of pretending it consumed cleanly. + assert rc == 1 From 1a6f4d0337b7883f3b9ecfccdc62f8ffe4ed1030 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Mon, 17 Aug 2026 10:06:02 +0000 Subject: [PATCH 3/3] collate_changelog: scope rerun folded-check to the target version's section A whole-file match let a late-landing fragment whose bullet duplicated an older release's line be unlinked as "already folded", silently losing the new note. Match only the target version's section (through the next version header). Red-proven: new test fails on the prior head, 9 pass with the fix. --- scripts/collate_changelog.py | 9 +++++++- tests/test_collate_changelog.py | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/scripts/collate_changelog.py b/scripts/collate_changelog.py index e5cc87f60..68a629d67 100644 --- a/scripts/collate_changelog.py +++ b/scripts/collate_changelog.py @@ -114,10 +114,17 @@ def main(argv: list[str]) -> int: # failed run is folded nowhere -- unlinking it would silently lose its # release note, so keep it and refuse loudly instead. print(f"collate-changelog: {args.version} section already present in {CHANGELOG.name}, consuming folded leftover fragments", file=sys.stderr) + # Match against ONLY the target version's section. A bullet that happens + # to also appear under an OLDER release must not count as folded -- + # unlinking on a whole-file match would silently lose the new note, + # which is the exact class this branch exists to prevent. + start = text.index(version_header) + next_header = text.find("\n## [", start + len(version_header)) + section_text = text[start:] if next_header == -1 else text[start:next_header] unfolded: list[Path] = [] for path in consumed: lines = [ln for section_lines in parse_fragment(path).values() for ln in section_lines] - if all(ln in text for ln in lines): + if all(ln in section_text for ln in lines): path.unlink() else: unfolded.append(path) diff --git a/tests/test_collate_changelog.py b/tests/test_collate_changelog.py index 84b3c530f..31f65838b 100644 --- a/tests/test_collate_changelog.py +++ b/tests/test_collate_changelog.py @@ -191,3 +191,42 @@ def fake_unlink(self): assert text.count("## [1.0.0-beta.47]") == 1 # And the rerun says NO loudly instead of pretending it consumed cleanly. assert rc == 1 + + +def test_rerun_keeps_unfolded_fragment_whose_text_matches_an_older_release(repo: Path, monkeypatch): + """A late-landing fragment whose bullet also exists under an OLDER release. + + The rerun's folded-check must scope its match to the target version's + section: a whole-file match sees the older release's identical bullet, + counts the fragment as folded, and unlinks it -- silently losing the new + release note. + """ + mod = _load(repo) + (repo / "changelog.d" / "2291-notes.md").write_text("- Notes area (#2291).\n", encoding="utf-8") + + fail_once = True + import pathlib + + _real_unlink = pathlib.Path.unlink + + def fake_unlink(self): + nonlocal fail_once + if fail_once: + fail_once = False + raise OSError("simulated unlink failure") + _real_unlink(self) + + monkeypatch.setattr(pathlib.Path, "unlink", fake_unlink) + + with pytest.raises(OSError, match="simulated unlink failure"): + mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) + + # A new PR merges a fragment whose text duplicates a bullet ALREADY present + # in the older 1.0.0-beta.46 section of the fixture changelog. + (repo / "changelog.d" / "2299-new.md").write_text("- older thing (#1).\n", encoding="utf-8") + + rc = mod.main(["1.0.0-beta.47", "--date", "2026-08-05"]) + + # The unfolded fragment survives and the rerun refuses loudly. + assert (repo / "changelog.d" / "2299-new.md").exists() + assert rc == 1