From 24ce8159634c517fa9637305f3a4f4780147c0b2 Mon Sep 17 00:00:00 2001 From: johanzander Date: Sat, 22 Aug 2026 15:28:32 +0200 Subject: [PATCH] Fix beta-release changelog merges absorbing the new section into the previous one The 3-way merge of beta/main into a release branch cannot express "append the new section onto beta's accumulated history", so hand-resolved conflicts repeatedly collapsed the new section into the previous one (#648). Add scripts/check-changelog.py: a `build` subcommand that resolves the conflict deterministically (beta/main's published history verbatim, with the new version section inserted after the preamble), and a `check` subcommand that asserts prepend-only, coverage, and no-re-announcing invariants, mapping merged PRs to the issues they reference via the merge-commit body. Wire both into the release skill's step 4/5. Refs #648 --- .claude/skills/release/SKILL.md | 34 +- CHANGELOG.md | 6 + backend/tests/test_check_changelog.py | 397 +++++++++++++++++++++++ scripts/check-changelog.py | 435 ++++++++++++++++++++++++++ 4 files changed, 869 insertions(+), 3 deletions(-) create mode 100644 backend/tests/test_check_changelog.py create mode 100644 scripts/check-changelog.py diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 6080b2a2..32998057 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -11,9 +11,20 @@ Commit as `git commit -am "release: v"`. Pushing this single commit (not raw `origin/main`) is what keeps the beta repo from ever momentarily claiming to be the prod add-on. 4. **Copy the changelog, don't author it** — on the same `beta-release-tmp` branch from step 3, take the current `## [Unreleased]` section verbatim from `origin/main`'s `CHANGELOG.md` (synced in step 1) and rename it to `## [] - ` in `CHANGELOG.md`. Amend it into the same commit (`git commit --amend`) rather than adding a second commit. - **First, detect PRs merged since the last beta with no changelog entry at all.** Get the previous beta release's publish timestamp (`gh release view v -R johanzander/bess-manager-beta --json publishedAt`), then list every `(#N)`-tagged merge commit on `origin/main` since it: `git log origin/main --oneline --since=""`. For each PR number found, grep `CHANGELOG.md`'s `Unreleased` section for a matching `#N` link. Any PR with no match is a real gap — it merged without a changelog entry, not just already-shipped content to curate away. Do not silently drop these: read the PR's diff/description (`gh pr view --repo johanzander/bess-manager`), judge whether it's user-facing (a real user would notice the behavior/UI change) or purely internal (refactor, CI wiring, doc/skill-only) — internal PRs correctly have no entry and need none — then for user-facing gaps draft a one-line entry in the existing style and present it in chat for confirmation. Once confirmed, add it both to `origin/main` (a small separate PR against `origin/main`'s `CHANGELOG.md` `Unreleased` section, since that file is the canonical source of truth for every future release, per `CLAUDE.md`) and to this beta's `## []` section. + **Let `scripts/check-changelog.py check` do the gap detection and the curation — don't hand-grep.** On this `beta-release-tmp` branch, run it against the section you just renamed: - **Then curate, don't dump.** `origin/main`'s `Unreleased` section only ever grows — it's cleared by a *stable* release, not a beta one — so by the second beta release it typically contains content already shipped in an earlier `bN`. Check each entry's PR against `git log --oneline origin/main | grep '(#N)'` relative to the previous beta release's sync-point commit (the last "chore: re-sync..." or release PR merge on `origin/main`); anything that merged *before* that point already shipped and must be dropped from this release's section, not re-listed. Keep only what's new since the last beta, plus a one-line note pointing at the previous release for context (see the `v9.9.0b10`/`v9.9.0b11` entries for the pattern). Getting this wrong silently double-announces old work as new in every subsequent release — it compounds. + ``` + scripts/check-changelog.py check --changelog CHANGELOG.md \ + --since-ref "$(git merge-base origin/main beta/main)" --section + ``` + + `--since-ref` is the previous beta's sync point — the merge-base of `origin/main` and `beta/main` — which is precise, unlike the previous release's *publish* timestamp (a PR can merge after the cut but before the release is published). `--section ` targets the renamed `## []` section explicitly rather than assuming it is the topmost `## [` heading (finding: after a stable release, the topmost section is the stable version). + + `check` asserts two invariants against that section (with `--beta-ref`/`--beta-file` omitted it skips the prepend-only check, which only makes sense after the step-5 merge): + - **coverage** — every `(#N)` merge commit on `origin/main` since the previous beta's sync point must appear as a `[#N](...)` link in this section — by its PR number or by an issue it references. A missing link is a real gap: it merged with no changelog entry, not just already-shipped content to curate away. Do not silently drop it. + - **no re-announcing** — every `[#N](...)` link in this section must correspond to a PR that merged *after* that same sync point (by its number or a referenced issue). Anything merged before already shipped in an earlier `bN` and must be dropped, not re-listed (see the `v9.9.0b10`/`v9.9.0b11` entries for the pattern). Getting this wrong silently double-announces old work as new in every subsequent release — it compounds. + + Iterate until `check` exits 0. When it flags a coverage gap, read the PR's diff/description (`gh pr view --repo johanzander/bess-manager`) and judge whether it's user-facing (a real user would notice the behavior/UI change) or purely internal (refactor, CI wiring, doc/skill-only) — internal PRs correctly have no entry and need none, so add their numbers to a comma-separated `--internal` list and re-run. For user-facing gaps, draft a one-line entry in the existing style and present it in chat for confirmation. Once confirmed, add it both to `origin/main` (a small separate PR against `origin/main`'s `CHANGELOG.md` `Unreleased` section, since that file is the canonical source of truth for every future release, per `CLAUDE.md`) and to this beta's `## []` section. 5. **Merge `beta/main` into this branch — expect exactly two conflicts, and that's normal, not an error:** ``` @@ -22,7 +33,24 @@ `beta/main`'s tip is never an ancestor of `origin/main` after the very first beta release (its own version-stamp commit only exists there), so this is never a fast-forward and a plain merge is the correct tool going forward — a failed `--ff-only` here does *not* mean the "beta never gets its own commits" rule was broken. Two conflicts are guaranteed by construction and mechanical to resolve: - `bess_manager/config.yaml`'s `version:` line — keep **ours** (the new `bN` this release just set in step 3). - - `CHANGELOG.md`'s heading — keep **ours** (the new version heading + step 4's curated entries), then make sure the previous release's already-published section (which `beta/main` has and this branch doesn't) still appears immediately below it. If your merge tool put it somewhere else or dropped it, fix that before committing — the historical section must survive. + - `CHANGELOG.md`'s heading — don't resolve it by hand. The 3-way merge cannot express "append step 4's new section onto beta's accumulated history", so it collapses the new section into the previous one (issue #648) or misplaces the historical section. Resolve deterministically instead — extract both conflicted sides, rebuild, then re-verify: + + ``` + git show :2:CHANGELOG.md > changelog-new.md + git show :3:CHANGELOG.md > changelog-beta.md + scripts/check-changelog.py build --new changelog-new.md --beta changelog-beta.md --out CHANGELOG.md + rm changelog-new.md changelog-beta.md + ``` + + `build` keeps beta/main's published history verbatim and inserts the new version section after the preamble — byte-stable, no section collapse. Then re-run the full invariant check against beta, now including prepend-only: + + ``` + scripts/check-changelog.py check --changelog CHANGELOG.md \ + --since-ref "$(git merge-base origin/main beta/main)" --section \ + --beta-ref beta/main --internal + ``` + + It must exit 0: coverage (every PR merged since the last beta is listed — by PR number or a referenced issue), no re-announcing (nothing already shipped is re-listed), and prepend-only (stripping the new section leaves beta/main's file byte-for-byte). If it fails, `build` was not the only resolution that happened — investigate before committing. Any *other* file conflicting is not expected and needs real investigation (usually: `origin/main` moved between when this branch's base was chosen and now, surfacing content `beta/main` hasn't seen — resolve by taking the newer, `origin/main`-based side, since that's always the more current code). Commit the resolution as `git commit -m "merge: reconcile beta/main history for v release"`. 6. **Run tests locally** — ALL of these must pass before proceeding: diff --git a/CHANGELOG.md b/CHANGELOG.md index bac95dcd..15416a04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to BESS Battery Manager will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **Beta release changelog merges no longer absorb the new section into the previous one** — the merge is now resolved deterministically instead of by hand. ([#648](https://github.com/johanzander/bess-manager/issues/648)) + ## [10.1.0] - 2026-08-22 ### Added diff --git a/backend/tests/test_check_changelog.py b/backend/tests/test_check_changelog.py new file mode 100644 index 00000000..ac4fe971 --- /dev/null +++ b/backend/tests/test_check_changelog.py @@ -0,0 +1,397 @@ +"""Tests for scripts/check-changelog.py — the beta-release CHANGELOG guard. + +The script has two jobs (wired into `.claude/skills/release/SKILL.md`): + +- ``build`` deterministically resolves the CHANGELOG.md merge conflict in the + beta release flow: take beta/main's published history verbatim and insert + the new section after the preamble, instead of letting git's 3-way merge + guess (which repeatedly absorbed the new section into the previous one, + #648). +- ``check`` asserts three invariants on the built file: + * prepend-only — stripping the new section leaves beta/main's published + history byte-identical (catches absorbed sections, dropped lines, silent + reflows); + * coverage — every PR merged on origin/main since the last beta appears in + the new section (by its PR number or an issue it references) or is + dismissed on the explicit internal list; + * no re-announcing — every PR linked in the new section merged after the + last beta's cut (nothing already shipped is re-listed). + +The CHANGELOG links issue numbers for most entries, so coverage and +no-re-announcing map merged PRs to their issues via the merge-commit body +(GitHub embeds the PR body there). + +The pure functions under test are the heart of the script; the CLI is a thin +git wrapper around them. Fixtures model a b13 release over a b12/b11 beta +history. +""" + +import importlib.util +import os +import stat +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / "scripts" / "check-changelog.py" + +# The script's filename carries the issue's exact name (`check-changelog.py`, +# hyphen), which Python cannot `import`; load it by path like test_app_startup +# does for app.py. +_spec = importlib.util.spec_from_file_location("check_changelog", SCRIPT) +assert _spec is not None and _spec.loader is not None +check_changelog = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(check_changelog) + +ChangelogCheckError = check_changelog.ChangelogCheckError +MergedPR = check_changelog.MergedPR +build_release_changelog = check_changelog.build_release_changelog +check_coverage = check_changelog.check_coverage +check_no_reannouncing = check_changelog.check_no_reannouncing +check_prepend_only = check_changelog.check_prepend_only +extract_new_section = check_changelog.extract_new_section +parse_merged_prs = check_changelog.parse_merged_prs +parse_pr_refs = check_changelog.parse_pr_refs +strip_new_section = check_changelog.strip_new_section + +# --- fixtures --------------------------------------------------------------- + +_ISSUE_URL = "https://github.com/johanzander/bess-manager/issues/{}" + + +def _entry(desc: str, n: int) -> str: + return f"- {desc}. ([#{n}]({_ISSUE_URL.format(n)}))\n" + + +PREAMBLE = "# Changelog\n\nIntro line.\n\n" + +# The new section a b13 release would carry (renamed Unreleased + curated). +NEW_SECTION = ( + "## [10.1.0b13] - 2026-08-22\n" + "\n" + "Delta from `v10.1.0b12`. Everything else accumulated in `Unreleased` on " + "main already shipped in `v10.1.0b12` or earlier; this release covers only " + "what is genuinely new since then.\n" + "\n" + "### Fixed\n" + "\n" + _entry("Entry A", 680) + "\n" +) + +# beta/main's published history: b12 (1 entry) then b11 (1 entry). +BETA_MAIN = ( + PREAMBLE + + "## [10.1.0b12] - 2026-08-21\n" + + "\n" + + "### Fixed\n" + + "\n" + + _entry("Entry X", 650) + + "\n" + + "## [10.1.0b11] - 2026-08-18\n" + + "\n" + + "### Fixed\n" + + "\n" + + _entry("Entry Y", 630) + + "\n" +) + +# The release branch's own CHANGELOG at merge-conflict time: the new section on +# top, then origin/main's (stable) history below — which beta already covers in +# its own beta sections and must be discarded in favour of beta/main's verbatim. +RELEASE_BRANCH = ( + PREAMBLE + + NEW_SECTION + + "## [10.0.2] - 2026-08-10\n" + + "\n" + + "### Fixed\n" + + "\n" + + _entry("Old stable entry", 600) + + "\n" +) + +# The deterministic resolution: preamble + new section + beta/main verbatim. +BUILT = PREAMBLE + NEW_SECTION + BETA_MAIN[len(PREAMBLE) :] + +# The exact corruption #648 describes: b13's entries absorbed into b12's +# section, no b13 heading at all. +ABSORBED = ( + PREAMBLE + + "## [10.1.0b12] - 2026-08-21\n" + + "\n" + + "### Fixed\n" + + "\n" + + _entry("Entry X", 650) + + _entry("Entry A", 680) + + "\n" + + "## [10.1.0b11] - 2026-08-18\n" + + "\n" + + "### Fixed\n" + + "\n" + + _entry("Entry Y", 630) + + "\n" +) + +# A dropped line from published history (b7's #512 line was lost this way). +DROPPED = BUILT.replace(_entry("Entry X", 650), "") + +# PRs merged on origin/main since b12's cut: #680 fixed #680, #690 fixed +# nothing tracked, #701 referenced #680 in its title. +MERGED_PRS = [ + MergedPR(680, frozenset({680})), + MergedPR(690, frozenset()), + MergedPR(701, frozenset({680})), +] + + +# --- section extraction / build -------------------------------------------- + + +def test_extract_new_section_takes_topmost_section() -> None: + assert extract_new_section(RELEASE_BRANCH) == NEW_SECTION + + +def test_extract_new_section_by_prefix() -> None: + assert extract_new_section(BUILT, section_prefix="10.1.0b13") == NEW_SECTION + + +def test_extract_new_section_prefix_missing_raises() -> None: + with pytest.raises(ChangelogCheckError): + extract_new_section(BUILT, section_prefix="10.1.0b99") + + +def test_build_is_deterministic() -> None: + assert build_release_changelog(RELEASE_BRANCH, BETA_MAIN) == BUILT + + +def test_build_roundtrips_strip_to_beta_main() -> None: + assert strip_new_section(BUILT) == BETA_MAIN + + +# --- prepend-only ----------------------------------------------------------- + + +def test_prepend_only_accepts_correctly_built_changelog() -> None: + check_prepend_only(BUILT, BETA_MAIN) # must not raise + + +def test_prepend_only_rejects_absorbed_section() -> None: + with pytest.raises(ChangelogCheckError): + check_prepend_only(ABSORBED, BETA_MAIN) + + +def test_prepend_only_rejects_dropped_line() -> None: + with pytest.raises(ChangelogCheckError): + check_prepend_only(DROPPED, BETA_MAIN) + + +# --- coverage --------------------------------------------------------------- + + +def test_coverage_flags_merged_pr_with_no_entry() -> None: + uncovered = check_coverage(NEW_SECTION, MERGED_PRS, internal={700}) + assert uncovered == [690] + + +def test_coverage_passes_when_every_pr_referenced_or_dismissed() -> None: + covered = [MergedPR(680, frozenset({680})), MergedPR(700, frozenset())] + assert check_coverage(NEW_SECTION, covered, internal={700}) == [] + + +def test_coverage_accepts_pr_that_references_a_linked_issue() -> None: + # #701 isn't linked directly, but it references #680 in its title, and + # #680 is the issue the entry links — the CHANGELOG convention. + assert check_coverage(NEW_SECTION, MERGED_PRS, internal=set()) == [690] + + +# --- no re-announcing ------------------------------------------------------- + + +def test_no_reannouncing_accepts_only_fresh_links() -> None: + assert check_no_reannouncing(NEW_SECTION, MERGED_PRS, dismiss=set()) == [] + + +def test_no_reannouncing_flags_already_shipped_link() -> None: + section = NEW_SECTION + _entry("Already shipped", 650) + assert check_no_reannouncing(section, MERGED_PRS, dismiss=set()) == [650] + + +def test_no_reannouncing_dismiss_exempts_a_kept_link() -> None: + section = NEW_SECTION + _entry("Kept deliberately", 650) + assert check_no_reannouncing(section, MERGED_PRS, dismiss={650}) == [] + + +def test_no_reannouncing_accepts_issue_backed_by_post_cut_pr() -> None: + # #555 is an old issue re-opened by a PR merged after the cut; the fresh + # entry legitimately links it. + section = NEW_SECTION + _entry("Reopened old issue", 555) + merged = [*MERGED_PRS, MergedPR(710, frozenset({555}))] + assert check_no_reannouncing(section, merged, dismiss=set()) == [] + + +# --- parsing ---------------------------------------------------------------- + + +def test_parse_pr_refs_ignores_bare_hash_refs_in_prose() -> None: + # The "Delta from" note references #450 in prose; only the [link](form) + # counts as an entry reference. + text = "the #450 PWL re-solve, plus " + _entry("linked", 512) + assert parse_pr_refs(text) == {512} + + +def test_parse_merged_prs_from_git_log_bodies() -> None: + # NUL-separated full bodies (git log --format=%B%x00). GitHub embeds the + # PR body, so "Closes #N" / "Refs #N" are the issue links. `release:` and + # `Merge ` commits are excluded automatically. + # + # Real git output pads each `--format` record with a trailing newline, so + # every block after the first begins with a blank line — this fixture + # reproduces that exactly (a regression: the parser used to take the first + # line of each block as the subject and silently dropped every commit + # after the first). + log = ( + "fix: survive a transient HA failure in the charging-power tick " + "(#643) (#675)\n\nCloses #643\n\x00\n" + "fix: report price health from the cache (#667)\n\n" + "With a cold probe still ERROR.\n\nCloses #662\n\x00\n" + "release: v10.1.0 (#674)\n\nRelease stamp.\n\x00\n" + "Merge pull request #104 from johanzander/foo\n\nMerged.\n\x00\n" + "feat: ship a widget (#701)\n\nAdds the widget.\n\x00\n" + ) + assert parse_merged_prs(log) == [ + MergedPR(675, frozenset({643})), + MergedPR(667, frozenset({662})), + MergedPR(701, frozenset()), + ] + + +# --- CLI -------------------------------------------------------------------- + + +def _run_cli(args: list[str], **extra_env: str) -> subprocess.CompletedProcess: + env = dict(os.environ, **extra_env) + return subprocess.run( + [sys.executable, str(SCRIPT), *args], capture_output=True, text=True, env=env + ) + + +def _write_git_shim(bin_dir: Path, beta_path: Path, log_path: Path) -> None: + shim = bin_dir / "git" + shim.write_text( + "#!/bin/sh\n" + 'if [ "$1" = "show" ]; then cat "$GIT_SHIM_BETA";\n' + 'elif [ "$1" = "log" ]; then cat "$GIT_SHIM_LOG";\n' + 'else echo "unhandled git $*" >&2; exit 1; fi\n' + ) + shim.chmod(shim.stat().st_mode | stat.S_IEXEC) + + +def test_build_cli_writes_deterministic_output(tmp_path: Path) -> None: + new = tmp_path / "new.md" + beta = tmp_path / "beta.md" + out = tmp_path / "out.md" + new.write_text(RELEASE_BRANCH) + beta.write_text(BETA_MAIN) + + proc = _run_cli( + ["build", "--new", str(new), "--beta", str(beta), "--out", str(out)] + ) + assert proc.returncode == 0, proc.stderr + assert out.read_text() == BUILT + + +def test_check_cli_accepts_clean_changelog(tmp_path: Path) -> None: + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + beta_path = tmp_path / "beta.md" + log_path = tmp_path / "log.txt" + beta_path.write_text(BETA_MAIN) + # --format=%B%x00 body: subject then body with "Closes #680". + log_path.write_text("fix: a merged PR (#680)\n\nCloses #680\n\x00") + _write_git_shim(bin_dir, beta_path, log_path) + + changelog = tmp_path / "changelog.md" + changelog.write_text(BUILT) + + proc = _run_cli( + [ + "check", + "--changelog", + str(changelog), + "--beta-ref", + "beta/main", + "--since", + "2026-08-21T00:00:00Z", + "--internal", + "700", + ], + PATH=f"{bin_dir}:{os.environ['PATH']}", + GIT_SHIM_BETA=str(beta_path), + GIT_SHIM_LOG=str(log_path), + ) + assert proc.returncode == 0, proc.stderr + assert "OK" in proc.stdout + assert "Internal no-entry list: #700" in proc.stdout + + +def test_check_cli_rejects_absorbed_section(tmp_path: Path) -> None: + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + beta_path = tmp_path / "beta.md" + log_path = tmp_path / "log.txt" + beta_path.write_text(BETA_MAIN) + log_path.write_text("fix: a merged PR (#680)\n\nCloses #680\n\x00") + _write_git_shim(bin_dir, beta_path, log_path) + + changelog = tmp_path / "changelog.md" + changelog.write_text(ABSORBED) + + proc = _run_cli( + [ + "check", + "--changelog", + str(changelog), + "--beta-ref", + "beta/main", + "--since", + "2026-08-21T00:00:00Z", + ], + PATH=f"{bin_dir}:{os.environ['PATH']}", + GIT_SHIM_BETA=str(beta_path), + GIT_SHIM_LOG=str(log_path), + ) + assert proc.returncode == 1 + assert "prepend-only" in proc.stderr + + +def test_check_cli_section_missing_is_clean_error(tmp_path: Path) -> None: + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + beta_path = tmp_path / "beta.md" + log_path = tmp_path / "log.txt" + beta_path.write_text(BETA_MAIN) + log_path.write_text("fix: a merged PR (#680)\n\nCloses #680\n\x00") + _write_git_shim(bin_dir, beta_path, log_path) + + changelog = tmp_path / "changelog.md" + changelog.write_text(ABSORBED) + + proc = _run_cli( + [ + "check", + "--changelog", + str(changelog), + "--beta-ref", + "beta/main", + "--since-ref", + "b2a0107c", + "--section", + "10.1.0b13", + ], + PATH=f"{bin_dir}:{os.environ['PATH']}", + GIT_SHIM_BETA=str(beta_path), + GIT_SHIM_LOG=str(log_path), + ) + assert proc.returncode == 1 + assert "no `## [` section whose heading starts with" in proc.stderr diff --git a/scripts/check-changelog.py b/scripts/check-changelog.py new file mode 100644 index 00000000..1d3ddd44 --- /dev/null +++ b/scripts/check-changelog.py @@ -0,0 +1,435 @@ +#!/usr/bin/env python3 +"""Guard the beta-release CHANGELOG against silent corruption. + +The beta release flow (`.claude/skills/release/SKILL.md`) merges `beta/main` +into a fresh release branch. `CHANGELOG.md` conflicts on every release, and +git's 3-way merge has repeatedly collapsed the newly-prepended section into +the previous one (b7/b8 and b9/b10 on the published beta history, #648). + +This script has two subcommands: + +``build`` + Resolve the CHANGELOG merge deterministically instead of by hand: take + `beta/main`'s published history verbatim and insert the new section (the + `## [` section identified by ``--section``, default the topmost one) of the + release branch's own CHANGELOG after the preamble. The result is + byte-deterministic — this removes the failure mode rather than detecting + it. + +``check`` + Assert three invariants on a CHANGELOG, each replacing a rule the release + skill used to describe in prose: + + 1. **Prepend-only** (needs ``--beta-ref``/``--beta-file``) — strip the new + section and the remainder must be byte-identical to `beta/main`'s + CHANGELOG. Catches absorbed sections, dropped lines, and silent reflows + anywhere in published history. + 2. **Coverage** (needs the cut) — every `(#N)` merge commit on + `origin/main` since the previous beta's cut must be represented in the + new section — by its own PR number or by an issue it references — or be + named on the explicit ``--internal`` list (which the script prints on + the record). `release:` commits and merge commits are excluded + automatically. + 3. **No re-announcing** (needs the cut) — every `#N` linked in the new + section must belong to a PR that merged after the previous beta's cut + (its number or one of its referenced issues). Nothing already shipped + may be re-listed. + + The CHANGELOG links issue numbers for most entries, not the PR that fixed + them, so the coverage and no-re-announcing checks map merged PRs to their + issues via the merge-commit body (GitHub embeds the PR body there). The + cut is the previous beta's branch point: ``--since-ref`` (a commit) or + ``--since`` (a timestamp); with neither it defaults to the merge-base of + `origin/main` and `beta/main`. + +Usage: + scripts/check-changelog.py build --new \\ + --beta [--out ] [--section ] + scripts/check-changelog.py check --changelog \\ + [--section ] \\ + [--since ] [--since-ref ] \\ + [--internal ] [--internal-file ] \\ + [--dismiss ] [--dismiss-file ] \\ + [--beta-ref ] [--beta-file ] [--remote ] +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from pathlib import Path +from typing import NamedTuple + +# PR number referenced as a markdown link `[#N](...)` — the entry format the +# CHANGELOG uses. A bare `#N` in prose (e.g. a "Delta from" note's context +# list) is deliberately not matched: a note is context, not an entry. +_PR_LINK_RE = re.compile(r"\[#(\d+)\]\(") +# PR number in a merge-commit subject `fix: ... (#667)`. +_PR_COMMIT_RE = re.compile(r"\(#(\d+)\)") +# Issue references in a merge-commit body (GitHub embeds the PR body there): +# `Closes #662`, `Fixes #N`, `Refs #N`, ... +_ISSUE_REF_RE = re.compile( + r"\b(?:closes|fixes|refs|resolves|addresses)\s+#(\d+)", re.IGNORECASE +) +# Subjects that never need a changelog entry: the release's own stamp, an +# actual merge commit, or the one-time beta reset. +_AUTO_EXCLUDE_RE = re.compile( + r"^(release:|Merge |chore: reset beta/main)", re.IGNORECASE +) + + +class ChangelogCheckError(Exception): + """Raised when a CHANGELOG invariant is violated.""" + + +class MergedPR(NamedTuple): + """A PR merged on origin/main since the cut, with the issues it references.""" + + number: int + issues: frozenset[int] = frozenset() + + +def _heading_line_indexes(lines: list[str]) -> list[int]: + return [i for i, line in enumerate(lines) if line.startswith("## [")] + + +def _splitlines_keepends(text: str) -> list[str]: + return text.splitlines(keepends=True) + + +def _section_bounds(lines: list[str], section_prefix: str | None) -> tuple[int, int]: + """Start/end line indexes of the section treated as "new". + + With *section_prefix*, the section whose heading starts with + ``## []``; otherwise the topmost ``## [` section. + """ + headings = _heading_line_indexes(lines) + if not headings: + raise ChangelogCheckError("no `## [` section heading found in CHANGELOG") + if section_prefix is None: + idx = headings[0] + else: + matches = [i for i in headings if lines[i].startswith(f"## [{section_prefix}]")] + if not matches: + raise ChangelogCheckError( + f"no `## [` section whose heading starts with " + f"`## [{section_prefix}]` found in CHANGELOG" + ) + idx = matches[0] + pos = headings.index(idx) + end = headings[pos + 1] if pos + 1 < len(headings) else len(lines) + return idx, end + + +def extract_new_section(changelog: str, section_prefix: str | None = None) -> str: + """The ``## [` section of *changelog* identified by *section_prefix*.""" + lines = _splitlines_keepends(changelog) + start, end = _section_bounds(lines, section_prefix) + return "".join(lines[start:end]) + + +def strip_new_section(changelog: str, section_prefix: str | None = None) -> str: + """*changelog* with its new section removed.""" + lines = _splitlines_keepends(changelog) + start, end = _section_bounds(lines, section_prefix) + return "".join(lines[:start] + lines[end:]) + + +def insert_new_section(beta_changelog: str, new_section: str) -> str: + """*beta_changelog* verbatim, with *new_section* inserted after the preamble.""" + lines = _splitlines_keepends(beta_changelog) + headings = _heading_line_indexes(lines) + if not headings: + # First beta on a beta repo with no published sections yet: append + # after the preamble. + return beta_changelog.rstrip("\n") + "\n\n" + new_section + insert_at = headings[0] + return "".join(lines[:insert_at]) + new_section + "".join(lines[insert_at:]) + + +def build_release_changelog( + release_changelog: str, beta_changelog: str, section_prefix: str | None = None +) -> str: + """Deterministically resolve the release-branch CHANGELOG. + + Takes the *section_prefix*-identified section of the release branch's own + *release_changelog* (the renamed `Unreleased` block) and inserts it into + *beta_changelog*, which is otherwise taken verbatim. + """ + return insert_new_section( + beta_changelog, extract_new_section(release_changelog, section_prefix) + ) + + +def parse_pr_refs(text: str) -> set[int]: + """PR/issue numbers referenced as ``[#N](...)`` links in *text*.""" + return {int(n) for n in _PR_LINK_RE.findall(text)} + + +def parse_merged_prs(git_log: str) -> list[MergedPR]: + """Parse ``git log --format=%B%x00`` output into merged PRs + issue refs. + + Each NUL-separated block is one commit's full body. The PR number is the + trailing ``(#N)`` in the subject (GitHub appends it); earlier ``(#N)`` in + the subject and ``Closes/Fixes/Refs/... #N`` in the body are issue refs. + """ + merged: list[MergedPR] = [] + for blob in git_log.split("\x00"): + if not blob.strip(): + continue + # git pads each `--format` record with a trailing newline, so blobs + # after the first start with a blank line; the subject is the first + # non-empty line. + nonempty = [line for line in blob.splitlines() if line.strip()] + if not nonempty: + continue + subject = nonempty[0] + if _AUTO_EXCLUDE_RE.match(subject): + continue + prs = [int(n) for n in _PR_COMMIT_RE.findall(subject)] + if not prs: + continue + number = prs[-1] + issues = {int(n) for n in prs[:-1]} + issues.update(int(n) for n in _ISSUE_REF_RE.findall(blob)) + merged.append(MergedPR(number, frozenset(issues))) + return merged + + +def check_prepend_only( + release_changelog: str, beta_changelog: str, section_prefix: str | None = None +) -> None: + """Raise unless stripping the new section leaves *beta_changelog* byte-identical.""" + remainder = strip_new_section(release_changelog, section_prefix) + if remainder != beta_changelog: + raise ChangelogCheckError( + "prepend-only violated: the CHANGELOG below the new section is not " + "byte-identical to beta/main's published history" + ) + + +def check_coverage( + new_section: str, merged_prs: list[MergedPR], internal: set[int] +) -> list[int]: + """PRs merged since the cut with no entry in the section (empty = pass). + + A merged PR is covered when its own number or any issue it references + appears as a `[#N](...)` link in *new_section*. The CHANGELOG links issue + numbers for most entries, so the PR-to-issue mapping from the merge-commit + body is what makes this check meaningful. + """ + section_links = parse_pr_refs(new_section) + uncovered: list[int] = [] + for pr in merged_prs: + if pr.number in internal: + continue + if ({pr.number} | pr.issues) & section_links: + continue + uncovered.append(pr.number) + return sorted(uncovered) + + +def check_no_reannouncing( + new_section: str, merged_prs: list[MergedPR], dismiss: set[int] +) -> list[int]: + """Links in the new section not backed by a post-cut PR (empty = pass). + + A link is re-announcing old work when neither its number nor any post-cut + merged PR references it — i.e. its work shipped before the cut. *dismiss* + exempts links the maintainer has explicitly decided to keep. + """ + section_links = parse_pr_refs(new_section) + referenced: set[int] = set() + for pr in merged_prs: + referenced.add(pr.number) + referenced.update(pr.issues) + return sorted((section_links - referenced) - dismiss) + + +def _run_git(args: list[str]) -> str: + proc = subprocess.run(["git", *args], capture_output=True, text=True) + if proc.returncode != 0: + raise ChangelogCheckError( + f"`git {' '.join(args)}` failed:\n{proc.stderr.strip()}" + ) + return proc.stdout + + +def _resolve_beta_changelog(beta_ref: str | None, beta_file: str | None) -> str | None: + if beta_ref is not None: + return _run_git(["show", f"{beta_ref}:CHANGELOG.md"]) + if beta_file is not None: + return Path(beta_file).read_text() + return None + + +def _merged_prs( + since: str | None, since_ref: str | None, remote: str +) -> list[MergedPR]: + """Merged PRs on *remote*/main since the cut, with their issue references. + + The cut is ``--since-ref`` (a commit), ``--since`` (a timestamp), or — with + neither — the merge-base of *remote*/main and `beta/main`, i.e. the point + the previous beta branched from main. + """ + if since_ref is not None: + log = _run_git(["log", f"{since_ref}..{remote}/main", "--format=%B%x00"]) + elif since is not None: + log = _run_git(["log", f"{remote}/main", f"--since={since}", "--format=%B%x00"]) + else: + base = _run_git(["merge-base", f"{remote}/main", "beta/main"]).strip() + log = _run_git(["log", f"{base}..{remote}/main", "--format=%B%x00"]) + return parse_merged_prs(log) + + +def _parse_number_list(value: str | None, file_path: str | None) -> set[int]: + numbers: set[int] = set() + if value: + numbers.update(int(part.strip()) for part in value.split(",") if part.strip()) + if file_path is not None: + for line in Path(file_path).read_text().splitlines(): + line = line.strip().lstrip("#").strip() + if line: + numbers.add(int(line)) + return numbers + + +def cmd_build(args: argparse.Namespace) -> int: + release = Path(args.new).read_text() + beta = Path(args.beta).read_text() + built = build_release_changelog(release, beta, args.section) + if args.out: + Path(args.out).write_text(built) + print(f"wrote {args.out}") + else: + sys.stdout.write(built) + return 0 + + +def cmd_check(args: argparse.Namespace) -> int: + changelog = Path(args.changelog).read_text() + beta = _resolve_beta_changelog(args.beta_ref, args.beta_file) + internal = _parse_number_list(args.internal, args.internal_file) + dismiss = _parse_number_list(args.dismiss, args.dismiss_file) + + violations: list[str] = [] + + if beta is not None: + try: + check_prepend_only(changelog, beta, args.section) + except ChangelogCheckError as exc: + violations.append(str(exc)) + + merged: list[MergedPR] | None = None + try: + merged = _merged_prs(args.since, args.since_ref, args.remote) + except ChangelogCheckError as exc: + print(f"note: coverage/no-re-announcing skipped ({exc})", file=sys.stderr) + + if merged is not None: + try: + new_section = extract_new_section(changelog, args.section) + except ChangelogCheckError as exc: + violations.append(str(exc)) + else: + uncovered = check_coverage(new_section, merged, internal) + if uncovered: + violations.append( + "coverage violated: merged since the last beta with no entry " + "and not dismissed: " + ", ".join(f"#{n}" for n in uncovered) + ) + reannounced = check_no_reannouncing(new_section, merged, dismiss) + if reannounced: + violations.append( + "no-re-announcing violated: linked in the new section but " + "merged before the cut (already shipped): " + + ", ".join(f"#{n}" for n in reannounced) + ) + + if internal: + print("Internal no-entry list: " + ", ".join(f"#{n}" for n in sorted(internal))) + if dismiss: + print("Dismissed links: " + ", ".join(f"#{n}" for n in sorted(dismiss))) + + if violations: + for violation in violations: + print(f"ERROR: {violation}", file=sys.stderr) + return 1 + print("check-changelog: OK") + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Guard the beta-release CHANGELOG against silent corruption." + ) + sub = parser.add_subparsers(dest="command", required=True) + + build_p = sub.add_parser( + "build", help="deterministically resolve the CHANGELOG merge" + ) + build_p.add_argument( + "--new", required=True, help="release branch's CHANGELOG (ours)" + ) + build_p.add_argument("--beta", required=True, help="beta/main's CHANGELOG (theirs)") + build_p.add_argument( + "--out", help="write the result to this path instead of stdout" + ) + build_p.add_argument( + "--section", help="new section heading prefix (default: topmost)" + ) + build_p.set_defaults(command="build") + + check_p = sub.add_parser("check", help="assert the CHANGELOG invariants") + check_p.add_argument("--changelog", required=True) + check_p.add_argument( + "--section", help="new section heading prefix (default: topmost)" + ) + check_p.add_argument("--since", help="git log --since value (previous beta's cut)") + check_p.add_argument( + "--since-ref", help="git ref/commit to cut the merged-PR range" + ) + check_p.add_argument( + "--internal", help="comma-separated PR numbers dismissed as internal" + ) + check_p.add_argument( + "--internal-file", help="file of dismissed PR numbers, one per line" + ) + check_p.add_argument( + "--dismiss", help="comma-separated link numbers exempt from no-re-announcing" + ) + check_p.add_argument( + "--dismiss-file", help="file of exempted link numbers, one per line" + ) + check_p.add_argument( + "--beta-ref", help="git ref whose CHANGELOG.md is beta/main's history" + ) + check_p.add_argument("--beta-file", help="path to beta/main's CHANGELOG.md") + check_p.add_argument( + "--remote", default="origin", help="remote for the git log (default origin)" + ) + check_p.set_defaults(command="check") + + args = parser.parse_args() + + if args.command == "check": + if ( + args.beta_ref is None + and args.beta_file is None + and args.since is None + and args.since_ref is None + ): + parser.error( + "check needs --beta-ref/--beta-file and/or --since/--since-ref " + "to verify anything" + ) + return cmd_check(args) + if args.command == "build": + return cmd_build(args) + raise AssertionError(f"unhandled command: {args.command}") + + +if __name__ == "__main__": + sys.exit(main())