diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d98eab3b..917bdbda 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,6 +78,27 @@ jobs: - name: CI run: | pixi run ci + # blooop/devlaunch#527. A branch rebased across a release cut files its entry + # inside the shipped section and git resolves it *cleanly*, so this is the + # only signal there is -- `MERGEABLE` and a green suite both say nothing about + # it. Pull requests only: on a push there is no base to be frozen against. + # + # The base commit is fetched explicitly at depth 1 rather than switching the + # checkout above to `fetch-depth: 0`, which would pull the whole history into + # every run of this job to serve one `git show`. + - name: A released section is not a place to file a new entry + if: github.event_name == 'pull_request' + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + git fetch --depth=1 origin "$BASE_SHA" + # `git show` writes nothing and exits non-zero if the path is absent at + # that commit, and `set -e` turns that into a failure rather than an + # empty file that would compare equal to anything. + git show "$BASE_SHA:CHANGELOG.md" > "$RUNNER_TEMP/base-CHANGELOG.md" + python3 scripts/changelog_frozen.py \ + "$RUNNER_TEMP/base-CHANGELOG.md" CHANGELOG.md # Flagged `python` since #294, so it is reported as what it is — the # harness, the doc guards and `scripts/` — rather than as "the project's # coverage". The shipped crates are the `rust` flag, uploaded by the diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ca941b..918fca61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **CI now refuses a pull request that files a new entry inside an already + released section of this file.** `## [Unreleased]` is a stable heading that a + release cut renames: it becomes `## [0.25.0] - 2026-08-28`, and a fresh empty + `[Unreleased]` is inserted above it. A branch cut before that release carries + its entry anchored by context under the old heading, which is now the release. + Git sees lines added below a heading that still exists, resolves the merge with + no conflict, and the pull request reports mergeable with every check green while + a shipped version quietly grows bullets describing fixes it never contained. + + It is not a lapse of attention. Over one afternoon it happened four times + independently to three people who did not know of each other, and twice inside a + single build. Every instance was caught by someone reading the diff, because + until now there was nothing else that could catch it. + + The rule is that a version section already present on the base branch must be + byte identical on the branch. Phrased that way it permits the one commit that + legitimately rewrites this file: a release cut adds a heading that was not there + and modifies none that was. The tempting phrasing, that the released portion of + the file is unchanged, is wrong on its own terms rather than merely + inconvenient, since it fails every release and so would be switched off before + it ever caught anything. + - **`dl --prune` now reclaims the Docker volumes of workspaces devpod has already forgotten.** Deleting a workspace through `dl` has removed its two named volumes since devlaunch#325, and the names come from devpod's own record of what it diff --git a/scripts/changelog_frozen.py b/scripts/changelog_frozen.py new file mode 100755 index 00000000..17fe43a5 --- /dev/null +++ b/scripts/changelog_frozen.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 +"""A version section that already exists on the base branch must be untouched. + +blooop/devlaunch#527. A branch cut before a release and rebased after it files +its new entry *inside the shipped section*, and git resolves that without a +conflict -- so `MERGEABLE` is green, every test passes, and a released version +quietly grows bullets describing fixes it did not contain. + +The mechanism is that `## [Unreleased]` is a *stable heading that gets renamed*. +A release cut turns it into `## [0.25.0] - 2026-08-28` and inserts a fresh empty +`[Unreleased]` above. A branch whose entry sits under the old heading is anchored +by context to text that is now the release, and from git's point of view the +branch merely added lines below a heading that still exists. There is nothing for +a merge to conflict on, which is why nothing catches it. + +It is not a slip that better attention fixes. Over one afternoon it happened four +times independently to three agents who did not know of each other, on branches +`wayfinder/devlaunch-{305,308,349,354}`, and twice in a single build on #346. Every +instance was caught by a human or an agent reading the diff. Nothing automated +caught any, because until this there was nothing to catch them. + +**Why the rule is phrased about sections rather than about a region of the file.** +The obvious form -- "the released portion of the file is unchanged" -- is wrong on +its own terms, not merely inconvenient: a release cut *necessarily* rewrites that +region, so the first thing the guard would ever fail is the one commit that is +definitionally correct. A guard whose opening act is a false positive on the +project's own release ritual is switched off before it catches anything. Keyed by +version instead, the release cut passes by construction: it adds a heading that +was not there and modifies none that was. + +Everything below the newest release is frozen, which is what a changelog is for. +Editing an old entry stays possible; it just has to be a visible, deliberate +override rather than a thing that happens to you during a rebase. + +Usage: + changelog_frozen.py + +Exits 0 when every version present in both is byte-identical, 1 otherwise, and +1 (never 0) when either side cannot be read or parsed -- see `sections`. +""" + +import difflib +import re +import sys +from pathlib import Path + +# `## [Unreleased]`, `## [0.25.0] - 2026-08-28`. The bracketed version is the key; +# the rest of the line is part of the body to compare, so that editing a release's +# date is caught by the same rule that catches editing its bullets. +HEADING = re.compile(r"^## \[([^\]]+)\]") + +# The one heading whose body is *expected* to differ on every branch: it is where +# a new entry is supposed to go. +MOVING = "Unreleased" + + +class Unparsable(Exception): + """The file is not a changelog this guard can reason about. + + Raised rather than returned, and never caught into a pass. A guard that + cannot read its input has not checked anything, and reporting that as + success is the same class of defect as the one it exists to find. + """ + + +def sections(text: str, where: str) -> dict[str, str]: + """Split a changelog into `{version: section text, heading included}`. + + Anything before the first heading -- the title and the Keep a Changelog + preamble -- is not a version and is dropped. It is not compared, because it + belongs to no release and a change to it is an ordinary edit. + + A version heading appearing twice raises. It would otherwise silently take + whichever copy came last, and a comparison whose subject is ambiguous is a + hole in exactly the place this guard is supposed to be solid. + """ + found: dict[str, list[str]] = {} + order: list[str] = [] + current: str | None = None + for line in text.splitlines(keepends=True): + match = HEADING.match(line) + if match: + current = match.group(1) + if current in found: + raise Unparsable(f"{where}: '## [{current}]' appears more than once") + found[current] = [] + order.append(current) + if current is not None: + found[current].append(line) + if not order: + raise Unparsable(f"{where}: no '## [version]' headings found at all") + return {version: "".join(lines) for version, lines in found.items()} + + +def read(path: Path) -> str: + try: + return path.read_text(encoding="utf-8") + except OSError as problem: + raise Unparsable(f"{path}: cannot be read ({problem})") from problem + + +def frozen_sections_differ(base: dict[str, str], head: dict[str, str]) -> list[str]: + """Report every version present in both whose text is not identical. + + Versions only on `head` are new -- that is a release cut, and the case the + rule is shaped to permit. Versions only on `base` are a deletion, which this + does not police: removing a section is loud in a diff and has never been the + silent failure. What is silent is a section growing, and that is what is + compared here. + """ + complaints = [] + for version, base_text in base.items(): + if version == MOVING or version not in head: + continue + head_text = head[version] + if head_text == base_text: + continue + diff = "".join( + difflib.unified_diff( + base_text.splitlines(keepends=True), + head_text.splitlines(keepends=True), + fromfile=f"base CHANGELOG.md ## [{version}]", + tofile=f"head CHANGELOG.md ## [{version}]", + ) + ) + complaints.append( + f"'## [{version}]' is already released and this branch changes it.\n\n" + f"{diff}\n" + f"If this entry describes a change that is not in {version} -- which is what a\n" + f"rebase across a release cut produces, cleanly and with nothing else red --\n" + f"move it up into '## [Unreleased]'. If you really do mean to edit a shipped\n" + f"section, say so in the pull request; this guard is meant to make that a\n" + f"decision rather than an accident.\n" + ) + return complaints + + +def main(argv: list[str]) -> int: + if len(argv) != 3: + print(f"usage: {Path(argv[0]).name} ", file=sys.stderr) + return 2 + try: + base = sections(read(Path(argv[1])), "base") + head = sections(read(Path(argv[2])), "head") + except Unparsable as problem: + print( + f"CHANGELOG.md could not be checked, so it is not passing: {problem}", file=sys.stderr + ) + return 1 + complaints = frozen_sections_differ(base, head) + for complaint in complaints: + print(complaint, file=sys.stderr) + if complaints: + return 1 + print(f"every released section is untouched ({len(base) - 1} compared)") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/test/test_changelog_frozen.py b/test/test_changelog_frozen.py new file mode 100644 index 00000000..026a06f7 --- /dev/null +++ b/test/test_changelog_frozen.py @@ -0,0 +1,208 @@ +"""The guard that notices an entry filed inside an already-shipped release. + +blooop/devlaunch#527. `## [Unreleased]` is a stable heading that a release cut +*renames*: it becomes `## [0.25.0] - 2026-08-28` and a fresh empty `[Unreleased]` +is inserted above it. A branch cut before that release carries its entry anchored, +by context, under the old heading -- which is now the release. Git sees lines added +below a heading that still exists, resolves it with no conflict, and the pull +request reports `MERGEABLE` with every check green while a shipped version grows +bullets describing fixes it never contained. + +That is why the fixture below performs the **actual merge** rather than writing a +bad changelog by hand. A hand-written fixture would only prove the parser rejects +input someone already knew was wrong. What has to be proved is that the *default +outcome of the ordinary operation* is wrong -- `a_clean_merge_across_a_release_cut_ +files_the_entry_inside_the_release` asserts git reports no conflict and the entry +still lands in the wrong section, and that is the whole claim of the ticket. + +The scale is why it is worth a job: four times in one afternoon, three agents, none +aware of the others (`wayfinder/devlaunch-{305,308,349,354}`), plus twice in a single +build on #346. Every instance was caught by someone reading the diff. + +The other half of the design is what the guard *permits*, tested by +`the_release_cut_that_creates_the_hazard_passes_the_guard`: the rule is keyed by +version, so a release cut adds a heading and modifies none, and passes untouched. +The tempting phrasing -- "the released portion of the file is unchanged" -- fails +that commit, and a guard whose first firing is a false positive on the project's +own release ritual does not survive to catch anything. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).parent.parent +GUARD = (ROOT / "scripts" / "changelog_frozen.py").resolve() +CI = ROOT / ".github" / "workflows" / "ci.yml" + +PREAMBLE = """# Changelog + +All notable changes to this project will be documented in this file. + +""" + +RELEASED = """## [0.1.0] - 2026-01-01 + +### Added + +- The first release. +""" + + +def changelog(unreleased: str, *rest: str) -> str: + return PREAMBLE + f"## [Unreleased]\n{unreleased}\n" + "\n".join((*rest, RELEASED)) + + +def run(*args: str, cwd: Path | None = None) -> subprocess.CompletedProcess: + return subprocess.run(args, cwd=cwd, capture_output=True, text=True, check=False) + + +def check(base: str, head: str, tmp_path: Path) -> subprocess.CompletedProcess: + (tmp_path / "base.md").write_text(base, encoding="utf-8") + (tmp_path / "head.md").write_text(head, encoding="utf-8") + return run( + sys.executable, + str(GUARD), + str(tmp_path / "base.md"), + str(tmp_path / "head.md"), + ) + + +def git(*args: str, cwd: Path) -> None: + done = run("git", *args, cwd=cwd) + assert done.returncode == 0, f"git {' '.join(args)} failed: {done.stderr}" + + +# Named through the decorator so the fixture function and the parameter that +# receives it are not one name in one module, which pylint reads as shadowing. +@pytest.fixture(name="repo") +def repo_before_the_release_cut(tmp_path: Path) -> Path: + """A repository at the moment before the release cut. + + `main` has an unreleased entry of its own, so the merge below is a realistic + one: both sides touched `[Unreleased]`, which is what makes the anchoring + interesting rather than a trivial fast-forward. + """ + work = tmp_path / "repo" + work.mkdir() + git("init", "-q", "-b", "main", cwd=work) + git("config", "user.email", "guard@example.invalid", cwd=work) + git("config", "user.name", "Guard Fixture", cwd=work) + (work / "CHANGELOG.md").write_text( + changelog("\n### Fixed\n\n- A fix that shipped in 0.2.0.\n"), encoding="utf-8" + ) + git("add", "CHANGELOG.md", cwd=work) + git("commit", "-qm", "before the cut", cwd=work) + return work + + +def test_a_clean_merge_across_a_release_cut_files_the_entry_inside_the_release(repo, tmp_path): + # The ticket's claim, reproduced rather than described. Branch and main both + # edit `[Unreleased]`; main then *renames* that heading by cutting 0.2.0. The + # merge is clean -- asserted, because a conflict here would mean git had a + # signal and the whole premise is that it has none -- and the branch's entry + # ends up under `## [0.2.0]`, describing a fix that release does not contain. + base_before = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + + git("checkout", "-q", "-b", "fix/thing", cwd=repo) + (repo / "CHANGELOG.md").write_text( + changelog( + "\n### Fixed\n\n- A fix that shipped in 0.2.0.\n- The branch's own fix, written later.\n" + ), + encoding="utf-8", + ) + git("commit", "-qam", "the branch's entry", cwd=repo) + + git("checkout", "-q", "main", cwd=repo) + (repo / "CHANGELOG.md").write_text( + changelog( + "\n", + "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- A fix that shipped in 0.2.0.\n", + ), + encoding="utf-8", + ) + git("commit", "-qam", "Cut 0.2.0", cwd=repo) + cut = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + + git("checkout", "-q", "fix/thing", cwd=repo) + merge = run("git", "merge", "--no-edit", "main", cwd=repo) + + assert merge.returncode == 0, ( + f"the premise is that git sees nothing to conflict on: {merge.stderr}" + ) + merged = (repo / "CHANGELOG.md").read_text(encoding="utf-8") + assert "- The branch's own fix, written later." in merged.split("## [0.2.0]")[1], ( + "the fixture is only interesting if the entry really did land inside the release" + ) + + done = check(cut, merged, tmp_path) + + assert done.returncode == 1 + assert "'## [0.2.0]' is already released and this branch changes it." in done.stderr + assert "The branch's own fix, written later." in done.stderr + assert base_before != cut, "sanity: the cut did rewrite the file" + + +def test_the_release_cut_that_creates_the_hazard_passes_the_guard(tmp_path): + # The case the rule is shaped to permit, and the reason it is keyed by version + # rather than by region. This commit rewrites most of the released portion of + # the file -- it renames `[Unreleased]` to `[0.2.0]` and inserts a new empty + # `[Unreleased]` -- and modifies no heading that already existed. "The released + # portion is unchanged" would fail here, on the one commit that is by + # definition correct. + base = changelog("\n### Fixed\n\n- Something.\n") + head = changelog("\n", "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- Something.\n") + + done = check(base, head, tmp_path) + + assert done.returncode == 0, done.stderr + + +def test_an_ordinary_entry_under_unreleased_passes(tmp_path): + base = changelog("\n") + head = changelog("\n### Fixed\n\n- The thing this pull request fixes.\n") + + done = check(base, head, tmp_path) + + assert done.returncode == 0, done.stderr + + +def test_editing_a_released_date_is_caught_by_the_same_rule(tmp_path): + # The heading line is part of the compared text, so a silently corrected date + # is the same defect as a silently added bullet: a claim about a release that + # was made after it shipped. + base = changelog("\n") + head = changelog("\n").replace("## [0.1.0] - 2026-01-01", "## [0.1.0] - 2026-01-02") + + done = check(base, head, tmp_path) + + assert done.returncode == 1 + assert "'## [0.1.0]'" in done.stderr + + +def test_a_changelog_it_cannot_parse_fails_rather_than_passes(tmp_path): + # A guard that cannot read its input has not checked anything. Reporting that + # as success is the same species of defect as the one it exists to find -- + # see #517, a merge verdict pinned to a check set that was never complete. + done = check(changelog("\n"), "no headings here at all\n", tmp_path) + + assert done.returncode == 1 + assert "not passing" in done.stderr + + +def test_a_duplicated_version_heading_is_refused_rather_than_resolved(tmp_path): + # Two `## [0.1.0]` headings would otherwise compare against whichever came + # last, which is a hole precisely where the guard has to be solid. + done = check(changelog("\n"), changelog("\n") + RELEASED, tmp_path) + + assert done.returncode == 1 + assert "appears more than once" in done.stderr + + +def test_ci_runs_the_guard_on_pull_requests(): + # A guard nothing invokes is a guard that is not running, which is the failure + # `test_bench_workflow.py` and `test_review_guard.py` both write about. + workflow = CI.read_text(encoding="utf-8") + assert "scripts/changelog_frozen.py" in workflow