From 72030511b99a8a4effbbb35f7c6ffedd32d87ae4 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Mon, 20 Jul 2026 20:00:12 +0000 Subject: [PATCH] fix: assert no shipped changelog heading is deleted or duplicated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release headings are append-only: the ceremony (#111) adds one and nothing in CONTRIBUTING's release flow ever removes one. Nothing asserted that. The arming rule (test/release.test.ts, rig#66) is narrow by design — it asks whether the TOP section agrees with package.json's version, about ONE heading, the one a PR is about to write under. It says nothing about the rest of the file, and cannot: "a heading disappeared" is not a property of a tree, it is a property of a DIFF. So an author adding an entry under '## Unreleased' who types OVER the heading below it instead of inserting above it produces a tree every existing guard calls green. git merges it cleanly — a one-line edit in a file nobody touched concurrently, no conflict, no signal. The shipped section's body is now sitting under '## Unreleased' and the version it belonged to has no section at all. It surfaces at the NEXT release, when release-notes.sh cannot find the section it extracts by heading, or worse republishes the absorbed prose. Ports box's changelog-monotonic.sh (box#122, caught in review of box#118) rather than reimplementing the invariant a third time in TypeScript, and keeps both halves. Containment catches a DELETED heading; it cannot catch a DUPLICATED one, because a duplicate is head-side surplus and base-minus-head is blind to extras on the head side. Uniqueness on HEAD is asserted alongside it, and that half matters more in cast than in box: release-notes.sh's awk has no `exit`, so `grab` re-arms on every matching '## ' line and two copies of a version heading make the published body ABSORB whatever sits between them — with the stranded entry dropped from the next release's notes too. (rig's extractor truncates instead; cast has the absorbing one.) The existing "double re-arm" test covers duplicate '## Unreleased' only, not duplicate VERSION headings, which are the ones that reach release-notes.sh. Wired into ci.yml as its own step so a red run names the invariant that broke; pull requests only, because on a push to main the merge base IS HEAD and the assert is vacuous; STRICT=1 with fetch-depth: 0 so a checkout that cannot reach the base ref fails loudly instead of skipping quietly forever. '## Unreleased' stays outside the guarded set — the arming rule owns that heading and the ceremony legitimately consumes it. Closes #133 Co-Authored-By: Claude Opus 4.8 --- .github/scripts/changelog-monotonic.sh | 219 +++++++++++++++++++++++++ .github/workflows/ci.yml | 27 +++ CHANGELOG.md | 29 ++++ test/release.test.ts | 198 ++++++++++++++++++++++ 4 files changed, 473 insertions(+) create mode 100755 .github/scripts/changelog-monotonic.sh diff --git a/.github/scripts/changelog-monotonic.sh b/.github/scripts/changelog-monotonic.sh new file mode 100755 index 0000000..1a3c55d --- /dev/null +++ b/.github/scripts/changelog-monotonic.sh @@ -0,0 +1,219 @@ +#!/usr/bin/env bash +set -euo pipefail + +# changelog-monotonic.sh [] [] — assert that no SHIPPED +# release heading was DELETED by this branch: the set of '^## X.Y.Z' headings +# on HEAD must be a SUPERSET of the set at the merge base. +# +# Ported from box (heavy-duty/box#122, caught in review of box#118) for #133, +# because cast's release-notes.sh carries the exact awk shape that made box#118 +# dangerous. The failure it exists to catch leaves no trace either. An author +# adding an entry under '## Unreleased' REPLACES the line below it instead of +# inserting above it: +# +# -## 0.1.1 — 2026-07-19 +# +## Unreleased +# + +# +### Fixed +# + +# +- **An entry** +# +# git merges that cleanly — it is a one-line edit inside a file nobody has +# touched concurrently — and the shipped section's whole body is silently +# absorbed into '## Unreleased'. 0.1.1 no longer HAS a section; the notes +# anchor release-notes.sh extracts by is gone, and the next release cut from +# that state republishes 0.1.1's prose as if it were new. +# +# The ARMING rule (test/release.test.ts, "the changelog is armed for the next +# entry (rig#66)") is green on exactly that tree, correctly: it asks only +# whether the TOP section agrees with package.json's version, and deleting +# '## 0.1.1' leaves '## Unreleased' on top. It is not wrong, it is narrow — it +# guards ONE heading, the one a PR is about to write under. This guards the +# REST of the file, the part no single tree can be asked about at all, because +# "a heading disappeared" is not a property of a tree — it is a property of a +# DIFF. +# +# The rule, and why it needs no tuning: release headings are APPEND-ONLY. The +# ceremony (#111) adds one and never removes one; nothing else in the +# documented flow (CONTRIBUTING.md, "Releasing") touches them. So SUPERSET is +# exact — it has no legitimate violation to carve an exception for. The stamp +# is covered for free: rewriting '## Unreleased' -> '## X.Y.Z — DATE' ADDS +# X.Y.Z and removes no X.Y.Z heading, because 'Unreleased' is not one. +# '## Unreleased' is deliberately NOT in the set this guards — the arming rule +# owns that heading, keyed on package.json's version, and the ceremony +# legitimately consumes it. +# +# A file of its own, NOT a clause inside the arming assertions, for three +# reasons. Its input is different (a git history, not two files). Its +# degradation is different (no base ref is a SKIP, not a failure). And the +# arming assertions run against constructed in-memory changelog strings that +# are not git repos at all — folding a git-dependent assert into them would +# make every one of those cases either skip or lie. Same discipline as +# release-notes.sh: its own file so test/release.test.ts can drive it. + +base_ref="${1:-${CHANGELOG_MONOTONIC_BASE:-origin/main}}" +changelog="${2:-CHANGELOG.md}" + +# Fail-closed switch: CI sets it, so a SKIP that would be a sensible local +# degradation becomes a red run there instead. A guard that can silently +# stop guarding is the failure shape this whole family of checks exists to +# refuse, so the skip path is loud and CI refuses to take it at all. +strict="${CHANGELOG_MONOTONIC_STRICT:-0}" + +skip() { + if [ "$strict" = "1" ]; then + echo "changelog-monotonic: $* — and CHANGELOG_MONOTONIC_STRICT=1, so this is a FAILURE, not a skip." >&2 + echo " CI sets STRICT because a guard that quietly stops guarding is worse than no guard." >&2 + echo " Fix the checkout, not this script: the base ref must be fetched (fetch-depth: 0)." >&2 + exit 1 + fi + echo "changelog-monotonic: SKIPPED — $*" + echo " (Nothing was checked. In CI this same condition is a hard failure.)" + exit 0 +} + +[ -f "$changelog" ] || { echo "changelog-monotonic: no such file: $changelog" >&2; exit 1; } + +git rev-parse --is-inside-work-tree >/dev/null 2>&1 \ + || skip "not inside a git work tree, so there is no history to compare against" + +git rev-parse --verify --quiet "$base_ref^{commit}" >/dev/null \ + || skip "base ref '$base_ref' does not resolve here (a shallow clone, or a fork checkout without the upstream remote)" + +merge_base="$(git merge-base "$base_ref" HEAD 2>/dev/null || true)" +[ -n "$merge_base" ] \ + || skip "no merge base between '$base_ref' and HEAD (unrelated histories, or a clone too shallow to reach one)" + +# The set of RELEASE headings: '## ...' where looks like a +# version. Field $2, the same split the arming rule and release-notes.sh use, +# so the three cannot disagree about what a section header is. 'Unreleased' +# fails the shape and is excluded by construction. +headings_raw() { + awk ' + /^## / && $2 ~ /^[0-9]+\.[0-9]+\.[0-9]+/ { print $2 } + ' +} +headings() { headings_raw | sort -u; } + +# The changelog may not exist at the merge base at all (the commit that adds +# it). Nothing to have deleted, so nothing to assert. +base_file="$(git show "$merge_base:$changelog" 2>/dev/null || true)" +[ -n "$base_file" ] || { + echo "changelog-monotonic: $changelog does not exist at the merge base ($(git rev-parse --short "$merge_base")) — nothing could have been deleted." + exit 0 +} + +# --- uniqueness on HEAD (the box#118 class) ---------------------------------- +# Containment catches a DELETED heading. It cannot catch a DUPLICATED one: the +# duplicate is head-side SURPLUS, and `comm -23` (base minus head) is blind to +# extras on the head side — with or without `sort -u`, base {0.1.1} minus head +# {0.1.1, 0.1.1} is empty. Multiset comparison does not close it either, for +# the same reason. The assert that does is uniqueness of version headings ON +# HEAD, kept alongside containment rather than replacing it. +# +# cast is the MORE exposed of the two repos here (#133). release-notes.sh +# extracts with: +# +# /^## / { grab = ($2 == ver); next } +# grab { print } +# +# There is no `exit`. `grab` re-arms on every matching '## ' line, so two +# '## 0.1.1' headings make the published body ABSORB whatever sits between the +# copies — and an entry stranded there is dropped from the NEXT release's notes +# as well. (rig's extractor has `if (found) exit`, so it truncates instead of +# absorbing — same class, milder symptom. cast has the absorbing one.) +# +# This is the shape box#118's bad rebase actually produced: two +# '## 0.8.0 — 2026-07-19' headings with the incoming entry between them. Every +# other guard stayed green — the arming rule happy (the top section was still +# right), tests and `bash -n` clean — while release-notes.sh re-armed its grab +# on the second heading and folded post-cut prose into the shipped release +# body. Note the arming rule's "double re-arm" case counts duplicate +# '## Unreleased' headings only; duplicate VERSION headings, the ones that +# reach release-notes.sh, are this script's. +# +# Nothing legitimate repeats a version heading: the ceremony stamps a NEW +# version, and 'Unreleased' fails the version shape and never reaches here. +dupes="$(headings_raw < "$changelog" | sort | uniq -d)" +if [ -n "$dupes" ]; then + { + echo "changelog-monotonic: $changelog has DUPLICATE release heading(s):" + echo + printf '%s\n' "$dupes" | sed 's/^/ ## /' + echo + cat <&2 + exit 1 +fi + +base_headings="$(printf '%s\n' "$base_file" | headings)" +head_headings="$(headings < "$changelog")" + +# comm -23: lines in the base set that are NOT in the head set — exactly the +# headings this branch removed. +missing="$(comm -23 <(printf '%s\n' "$base_headings") <(printf '%s\n' "$head_headings"))" + +if [ -n "$missing" ]; then + { + echo "changelog-monotonic: this branch DELETES release heading(s) from $changelog:" + echo + printf '%s\n' "$missing" | sed 's/^/ ## /' + echo + cat <&2 + exit 1 +fi + +count="$(printf '%s\n' "$base_headings" | grep -c . || true)" +echo "changelog-monotonic: all $count release heading(s) at the merge base ($(git rev-parse --short "$merge_base")) are still present in $changelog" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91e357c..17934d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,18 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + # fetch-depth: 0, for the changelog-monotonic step below and only + # for it. That check is about a DIFF — which release headings the + # merge base had — so it needs the base branch's history present, + # and the default depth-1 checkout has none of it. An explicit + # `git fetch origin ` would be narrower, but it has to be + # right on both event types and on fork PRs, and getting it subtly + # wrong degrades to a SKIP (a guard that silently stops guarding — + # the exact failure this repo keeps refusing). Full history on a + # tree this size costs a second; the STRICT flag below turns any + # remaining skip red rather than green. + fetch-depth: 0 - uses: actions/setup-node@v4 with: node-version: "22" @@ -25,6 +37,21 @@ jobs: - name: labels state-machine tests run: bash test/labels-reconcile.sh + # ...and no SHIPPED release heading was deleted (#133; box#122's guard). + # Its own step so that when it goes red the log names the invariant that + # broke — and a DIFFERENT invariant from the arming rule npm test + # carries: arming is a fact about this tree, monotonicity is a fact + # about this tree versus its merge base. Pull requests only: on a push + # to main the merge base IS HEAD, so the assert is vacuous and would + # only add a green step that proves nothing. STRICT=1 so a checkout that + # cannot reach the base ref fails here instead of skipping quietly + # forever. + - name: no shipped changelog heading was deleted + if: github.event_name == 'pull_request' + env: + CHANGELOG_MONOTONIC_STRICT: "1" + run: bash .github/scripts/changelog-monotonic.sh "origin/${{ github.base_ref }}" + # The installer, proven by RUNNING it — CAST_INSTALL_SOURCE points it at # this checkout, so CI proves the installer under review (the versioned # layout, the current symlink, the PATH chain, the uninstall's absence diff --git a/CHANGELOG.md b/CHANGELOG.md index af11411..daf4615 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,35 @@ actually cutting it, and this file starts there. ### Fixed +- **A PR that deletes a shipped release heading is now CI-red** (#133, + heavy-duty/box#122) — `.github/scripts/changelog-monotonic.sh` asserts that + the set of `## X.Y.Z` headings on HEAD is a superset of the set at the merge + base, and that no version heading appears twice. Wired into `ci.yml` on pull + requests only (on a push to main the merge base *is* HEAD, so the assert is + vacuous), with `CHANGELOG_MONOTONIC_STRICT=1` and `fetch-depth: 0` so a + checkout that cannot reach the base ref fails loudly rather than skipping + quietly forever. + + The failure it catches leaves no trace. An author adding an entry under + `## Unreleased` types *over* the heading below it instead of inserting above + it — a one-line edit, in a file nobody touched concurrently, so git merges it + cleanly with no conflict and no signal. The arming rule stays green and is + not wrong to: the top section is still the right one for the version. But + the shipped section's body is now sitting under `## Unreleased`, and the + version it belonged to has no section at all. Nothing surfaces until the + *next* release, when `release-notes.sh` cannot find the section it extracts + by heading — or worse, republishes the absorbed prose as if it were new. + + The uniqueness half matters more here than in box. `release-notes.sh`'s awk + has no `exit`, so `grab` re-arms on every matching `## ` line: two + `## 0.1.1` headings make the published body **absorb** whatever sits between + the copies, and an entry stranded there is dropped from the next release's + notes as well. Containment alone cannot see it — a duplicate is head-side + surplus, and base-minus-head is blind to extras on the head side — so + uniqueness on HEAD is asserted alongside it. `## Unreleased` is deliberately + outside the guarded set: the arming rule owns that heading, and the ceremony + legitimately consumes it. + - **A label the repo does not have no longer takes the whole edit down with it** — `gh issue edit --add-label` rejects the *entire* call on one unknown name, applying nothing. Batching state and blockers into a single edit (for diff --git a/test/release.test.ts b/test/release.test.ts index 38f699c..0bf61e7 100644 --- a/test/release.test.ts +++ b/test/release.test.ts @@ -26,15 +26,18 @@ import { describe, expect, it } from "vitest"; const ROOT = dirname(dirname(fileURLToPath(import.meta.url))); const NOTES = join(ROOT, ".github/scripts/release-notes.sh"); +const MONOTONIC = join(ROOT, ".github/scripts/changelog-monotonic.sh"); function run( cmd: string, args: string[], env: Record = {}, + cwd?: string, ): Promise<{ code: number; output: string }> { return new Promise((resolve) => { const child = spawn(cmd, args, { stdio: ["ignore", "pipe", "pipe"], + cwd, env: { ...process.env, ...env }, }); let output = ""; @@ -376,6 +379,201 @@ describe("the changelog is armed for the next entry (rig#66)", () => { }); }); +// --- no SHIPPED release heading was deleted (#133) -------------------------- +// The arming block above guards ONE heading — the top one, the one a PR is +// about to write under — and is keyed on a single tree. This guards the REST +// of the file, which no single tree can be asked about: "a heading +// disappeared" is not a property of a tree, it is a property of a DIFF. So the +// fixtures here are real throwaway git repos with a base branch and a PR +// branch, and the assertions drive the REAL script the CI step runs, the same +// way the block above drives the real release-notes.sh. +// +// Two halves, and they catch different shapes. CONTAINMENT catches a DELETED +// heading (base's set must be a subset of HEAD's). It cannot catch a +// DUPLICATED one — a duplicate is head-side SURPLUS, and base-minus-head is +// blind to extras on the head side — so UNIQUENESS on HEAD is asserted +// alongside it. The duplicate half matters more in cast than in box: +// release-notes.sh has no `exit`, so `grab` re-arms on every matching '## ' +// line and two copies of a version heading make the published body ABSORB +// whatever sits between them. + +describe("changelog-monotonic.sh — release headings are append-only (#133)", () => { + const dated = (v: string) => `## ${v} — 2026-07-19`; + const body = "\n\n- **An entry** — prose.\n"; + /** The base branch's changelog: two shipped releases under an Unreleased. */ + const BASE = `# Changelog\n\n## Unreleased\n\n${dated("0.1.1")}${body}\n${dated("0.1.0")}${body}`; + + const git = (repo: string, ...args: string[]) => + execFileSync("git", args, { cwd: repo, encoding: "utf8" }); + + /** + * A throwaway repo with `base` carrying BASE, checked out on a PR branch + * whose CHANGELOG.md is `head` (unchanged when omitted). + */ + function repoWith(head?: string): string { + const repo = mkdtempSync(join(tmpdir(), "cast-monotonic-")); + git(repo, "init", "-q"); + git(repo, "config", "user.email", "test@example.com"); + git(repo, "config", "user.name", "test"); + git(repo, "checkout", "-q", "-b", "base"); + writeFileSync(join(repo, "CHANGELOG.md"), BASE); + git(repo, "add", "CHANGELOG.md"); + git(repo, "commit", "-qm", "base"); + git(repo, "checkout", "-q", "-b", "pr"); + if (head !== undefined) { + writeFileSync(join(repo, "CHANGELOG.md"), head); + git(repo, "add", "CHANGELOG.md"); + git(repo, "commit", "-qm", "the PR"); + } + return repo; + } + + const check = ( + repo: string, + env: Record = {}, + base = "base", + ) => run("bash", [MONOTONIC, base], env, repo); + + it("a branch that touches nothing passes, and says how many headings it checked", async () => { + const r = await check(repoWith()); + expect(r.code).toBe(0); + expect(r.output).toContain("all 2 release heading(s)"); + }); + + it("adding an entry the CORRECT way — above the heading, never over it — passes", async () => { + const good = BASE.replace( + "## Unreleased\n", + "## Unreleased\n\n### Fixed\n\n- **A new entry**\n", + ); + const r = await check(repoWith(good)); + expect(r.code).toBe(0); + }); + + it("goes RED when an entry REPLACED the shipped heading below it — the #133 failure, exactly", async () => { + // The one-line edit git merges cleanly and nothing else notices: the + // author typed over `## 0.1.1 — …` instead of inserting above it. + const clobbered = BASE.replace( + `${dated("0.1.1")}`, + "## Unreleased\n\n### Fixed\n\n- **An entry**", + ); + const r = await check(repoWith(clobbered)); + expect(r.code).toBe(1); + expect(r.output).toContain("DELETES release heading(s)"); + expect(r.output).toContain("## 0.1.1"); + expect(r.output).toContain("APPEND-ONLY"); + // 0.1.0, untouched, must not be accused. + expect(r.output).not.toContain(" ## 0.1.0"); + }); + + it("goes RED on a DUPLICATED version heading — the case containment cannot see", async () => { + // Head-side surplus: base {0.1.0, 0.1.1} minus head is still empty, so + // only the uniqueness half catches this. It is also the case the arming + // rule's "double re-arm" test does NOT cover — that one counts duplicate + // '## Unreleased' headings, not duplicate VERSION headings, and it is the + // version ones that reach release-notes.sh. + const twice = BASE.replace( + `${dated("0.1.1")}${body}`, + `${dated("0.1.1")}${body}\n${dated("0.1.1")}${body}`, + ); + const r = await check(repoWith(twice)); + expect(r.code).toBe(1); + expect(r.output).toContain("DUPLICATE release heading(s)"); + expect(r.output).toContain("## 0.1.1"); + expect(r.output).toContain("absorbs"); + }); + + it("the duplicate half survives the entry sitting BETWEEN the copies — the absorbing shape", async () => { + // What release-notes.sh would publish for 0.1.1 if this landed: its own + // prose, the stranded entry, AND the second copy's prose. Asserted with + // the real extractor, so the consequence is the actual one. + const absorbing = BASE.replace( + `${dated("0.1.1")}${body}`, + `${dated("0.1.1")}${body}\n- **A stranded entry**\n\n${dated("0.1.1")}${body}`, + ); + const repo = repoWith(absorbing); + const r = await check(repo); + expect(r.code).toBe(1); + const notes = await run("bash", [ + NOTES, + "0.1.1", + join(repo, "CHANGELOG.md"), + ]); + expect(notes.output).toContain("A stranded entry"); + }); + + it("'## Unreleased' is NOT in the guarded set — the ceremony legitimately consumes it", async () => { + // The release stamp: Unreleased becomes 0.2.0. That ADDS a version + // heading and removes none, and the Unreleased that disappeared is not a + // version heading at all. The arming rule owns that one. + const stamped = `${BASE.replace("## Unreleased\n", `${dated("0.2.0")}${body}\n`)}`; + const r = await check(repoWith(stamped)); + expect(r.code).toBe(0); + // And deleting Unreleased outright — a disarmed tree, red under the + // arming rule — is still not this guard's business. + const disarmed = BASE.replace("## Unreleased\n\n", ""); + expect((await check(repoWith(disarmed))).code).toBe(0); + }); + + it("a changelog absent at the merge base is nothing-to-have-deleted, not a failure", async () => { + const repo = mkdtempSync(join(tmpdir(), "cast-monotonic-new-")); + git(repo, "init", "-q"); + git(repo, "config", "user.email", "test@example.com"); + git(repo, "config", "user.name", "test"); + git(repo, "checkout", "-q", "-b", "base"); + writeFileSync(join(repo, "README.md"), "# hi\n"); + git(repo, "add", "README.md"); + git(repo, "commit", "-qm", "base"); + git(repo, "checkout", "-q", "-b", "pr"); + writeFileSync(join(repo, "CHANGELOG.md"), BASE); + git(repo, "add", "CHANGELOG.md"); + git(repo, "commit", "-qm", "add the changelog"); + const r = await check(repo); + expect(r.code).toBe(0); + expect(r.output).toContain("does not exist at the merge base"); + }); + + it("a missing changelog refuses by path — never a silent pass", async () => { + const r = await run("bash", [MONOTONIC, "base", "nope.md"], {}, repoWith()); + expect(r.code).toBe(1); + expect(r.output).toContain("no such file"); + }); + + // The fail-closed switch, both directions. A guard that can quietly stop + // guarding is the failure shape this whole family of checks refuses, so the + // degradation that is sensible locally must be RED in CI. + it("an unresolvable base ref is a SKIP locally", async () => { + const r = await check(repoWith(), {}, "origin/no-such-branch"); + expect(r.code).toBe(0); + expect(r.output).toContain("SKIPPED"); + expect(r.output).toContain("Nothing was checked"); + }); + + it("...and the SAME condition is a hard FAILURE under STRICT=1, naming fetch-depth", async () => { + const r = await check( + repoWith(), + { CHANGELOG_MONOTONIC_STRICT: "1" }, + "origin/no-such-branch", + ); + expect(r.code).toBe(1); + expect(r.output).toContain("CHANGELOG_MONOTONIC_STRICT=1"); + expect(r.output).toContain("fetch-depth: 0"); + expect(r.output).not.toContain("SKIPPED"); + }); + + // The wiring, pinned the same way release.yml's is — the script existing is + // no use if CI stops running it, and every clause here is load-bearing. + it("ci.yml runs it on pull requests only, STRICT, against the base ref, with full history", () => { + const CI = readFileSync(join(ROOT, ".github/workflows/ci.yml"), "utf8"); + expect(CI).toContain(".github/scripts/changelog-monotonic.sh"); + expect(CI).toContain('"origin/${{ github.base_ref }}"'); + // Pull requests only: on a push to main the merge base IS HEAD. + expect(CI).toContain("if: github.event_name == 'pull_request'"); + expect(CI).toContain("CHANGELOG_MONOTONIC_STRICT"); + // ...which is only reachable because the checkout has the base history. + expect(CI).toContain("fetch-depth: 0"); + }); +}); + // --- release.yml — the wiring, pinned -------------------------------------- // The workflow itself only runs on a tag push upstream, so its load-bearing // pieces are pinned here, fail-closed (the house discipline: the labels