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..5cfbe2244 --- /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. 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 a7da26837..68a629d67 100644 --- a/scripts/collate_changelog.py +++ b/scripts/collate_changelog.py @@ -106,6 +106,35 @@ 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: + # 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) + # 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 section_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, # 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..31f65838b 100644 --- a/tests/test_collate_changelog.py +++ b/tests/test_collate_changelog.py @@ -114,3 +114,119 @@ 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() + + +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 + + +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