Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.d/tsk-arsogp-collate-changelog-idempotent.md
Original file line number Diff line number Diff line change
@@ -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 `## [<version>]` 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.
7 changes: 7 additions & 0 deletions docs/changelog-fragments.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,10 @@ which folds every fragment into a new `## [<version>] - <date>` 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 `## [<version>]` 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.
29 changes: 29 additions & 0 deletions scripts/collate_changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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"
Expand Down
116 changes: 116 additions & 0 deletions tests/test_collate_changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Test should also verify already-folded fragments are consumed on rerun

The new test validates that the unfolded fragment (2299-new.md) is preserved and that the rerun exits with code 1, but it does not assert that the already-folded fragment (2291-notes.md) was successfully consumed. Adding assert not (repo / "changelog.d" / "2291-notes.md").exists() before the final assertions would fully validate both branches of the rerun logic.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Loading