fix: assert no shipped changelog heading is deleted or duplicated #134

Merged
dan-claude-bot merged 5 commits from fix/changelog-monotonic into main 2026-07-20 23:37:02 +00:00
4 changed files with 190 additions and 45 deletions
Showing only changes of commit 0bd531042e - Show all commits

View file

@ -64,26 +64,18 @@ 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 " (Uniqueness on HEAD already passed; it is containment that cannot run.)" >&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.)"
echo "changelog-monotonic: containment SKIPPED — $*"
echo " (Uniqueness on HEAD already ran and passed — only the deleted-heading"
echo " half needs the history. 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: '## <token> ...' where <token> 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'
@ -95,14 +87,6 @@ headings_raw() {
}
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
@ -168,6 +152,40 @@ EOF
exit 1
fi
# --- everything below needs the HISTORY --------------------------------------
# Uniqueness is settled. What follows is containment, which compares HEAD
# against the merge base and therefore genuinely depends on the base ref, the
# merge base, and the base blob. Each of those can be unavailable for reasons
# that are not the author's fault (a shallow clone, a fork checkout without the
# upstream remote, the commit that first adds the changelog), so each degrades
# rather than failing — which is exactly why the uniqueness half must NOT live
# down here (#133, box#143). It asks nothing of the history, and gating it
# behind these conditions let a duplicate exit 0 on a message about deletion.
#
# That ordering mattered MORE here than anywhere. cast's release-notes.sh has
# no `exit`, so `grab` re-arms on every matching '## ' line and a duplicate
# makes the published body ABSORB whatever sits between the copies — the live
# extraction bug this guard exists for. The half with that bug behind it was
# the half with the most ways to silently not run.
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 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 (uniqueness on HEAD already passed)."
exit 0
}
base_headings="$(printf '%s\n' "$base_file" | headings)"
head_headings="$(headings < "$changelog")"

View file

@ -37,20 +37,31 @@ 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'
# ...and no SHIPPED release heading was deleted or DUPLICATED (#133;
# box#122's guard, box#143's ordering fix). 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. STRICT=1 so a checkout that cannot reach the base ref
# fails here instead of skipping quietly forever.
#
# NOT pull-request-only, and that is the #133 fix at the workflow level.
# The two halves have different vacuity: DELETION is vacuous on a push
# to main (the merge base IS HEAD), but DUPLICATION is vacuous on no
# tree at all, so gating the whole script on `pull_request` left a
# duplicate that reached main by any other route unasserted forever.
#
# The `|| github.ref_name` fallback is load-bearing, not defensive. On a
# push event `github.base_ref` is EMPTY, so the argument would collapse
# to a bare `origin/`, which does not resolve — and STRICT=1 correctly
# promotes that to a hard failure, turning every push to main red. With
# the fallback it resolves to the pushed branch, whose merge base with
# HEAD is HEAD or its parent: containment passes vacuously, exactly as
# the old `if` intended, while uniqueness now runs on every push.
- name: no shipped changelog heading was deleted or duplicated
env:
CHANGELOG_MONOTONIC_STRICT: "1"
run: bash .github/scripts/changelog-monotonic.sh "origin/${{ github.base_ref }}"
run: bash .github/scripts/changelog-monotonic.sh "origin/${{ github.base_ref || github.ref_name }}"
# The installer, proven by RUNNING it — CAST_INSTALL_SOURCE points it at
# this checkout, so CI proves the installer under review (the versioned

View file

@ -52,12 +52,46 @@ actually cutting it, and this file starts there.
- **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
base, and that no version heading appears twice. Wired into `ci.yml` on
every event, 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.
- **...and a duplicate heading no longer slips through on the paths where the
guard cannot see the base** (#133, heavy-duty/box#143) — the uniqueness half
is a property of HEAD alone, but it sat downstream of the base-ref,
merge-base and base-blob conditions, so each of those degradations returned
success on a tree with a duplicate in plain sight.
The base-blob case was the worst of the three because it was not a skip at
all: a branch that *introduces* `CHANGELOG.md` exited 0 through a bare
`exit 0`, on a message that was true about deletion and silent about the
duplicate in front of it. `STRICT=1` could not reach it — STRICT guards the
two `skip()` calls, and that path is not one of them. Off CI the two skips
had the same shape, so a shallow clone or an unpacked tarball would not look
at a duplicate the author was about to push.
That inverted the two halves, and it inverted them hardest here. Deletion is
the failure that needs a diff to see; duplication is the one cast's
`release-notes.sh` actually mis-renders, and cast has the ABSORBING
extractor — no `exit`, so `grab` re-arms on the second heading and the
published body swallows whatever sits between the copies (heavy-duty/box#118).
The half with the live extraction bug behind it was the half with the most
ways to silently not run.
Fixed by moving, not rewriting: uniqueness now runs directly after the file
exists, before any git access. The skip messages say *containment* skipped
and that uniqueness already passed, so a skip no longer claims nothing was
checked. The guard is also no longer gated to `pull_request` — deletion is
vacuous on a push to main, but duplication is vacuous on no tree, so a
duplicate reaching main by any other route went unasserted. That gate could
not simply be dropped: `github.base_ref` is empty on a push, and a bare
`origin/` under `STRICT=1` is a hard failure on every push to main, so the
base ref falls back to `github.ref_name`.
Found by `claude-bot-andresmgsl` and `codex-bot-andresmgsl` reviewing #134;
cast inherited the ordering from box, fixed there in heavy-duty/box#144.
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

View file

@ -514,7 +514,12 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
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 () => {
/**
* A repo whose `base` has NO changelog at all the PR INTRODUCES the file.
* The merge-base blob is absent, which is the degradation path that used to
* `exit 0` before uniqueness had run (#133, box#143).
*/
function repoIntroducing(head: string): string {
const repo = mkdtempSync(join(tmpdir(), "cast-monotonic-new-"));
git(repo, "init", "-q");
git(repo, "config", "user.email", "test@example.com");
@ -524,14 +529,82 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
git(repo, "add", "README.md");
git(repo, "commit", "-qm", "base");
git(repo, "checkout", "-q", "-b", "pr");
writeFileSync(join(repo, "CHANGELOG.md"), BASE);
writeFileSync(join(repo, "CHANGELOG.md"), head);
git(repo, "add", "CHANGELOG.md");
git(repo, "commit", "-qm", "add the changelog");
const r = await check(repo);
return repo;
}
it("a changelog absent at the merge base is nothing-to-have-deleted, not a failure", async () => {
const r = await check(repoIntroducing(BASE));
expect(r.code).toBe(0);
expect(r.output).toContain("does not exist at the merge base");
});
// --- #133: uniqueness is a property of HEAD, so nothing base-side may gate
// it. Containment needs the merge base; uniqueness needs only the file in
// front of it. Before this fix the duplicate check sat DOWNSTREAM of the
// base-ref, merge-base and base-blob conditions, so each of the degradation
// paths below exited 0 on a tree carrying a duplicate in plain sight — the
// base-blob one not even via skip(), but a bare `exit 0` that STRICT could
// not reach. These cases pin the ORDER, which is the actual invariant;
// asserting the exit code alone is what let the original ship (the
// base-absent case above was green before and after).
//
// The inversion mattered most here: cast's release-notes.sh re-arms `grab`
// on every '## ' line, so duplication is the half with a LIVE extraction bug
// behind it — and it was the half with the most ways to silently not run.
it("a duplicate introduced where the base had NO changelog is caught (#133)", async () => {
const dup = `# Changelog\n\n## Unreleased\n\n${dated("0.1.1")}${body}\n- **A stranded entry**\n\n${dated("0.1.1")}${body}`;
const r = await check(repoIntroducing(dup));
expect(r.code).toBe(1);
expect(r.output).toContain("DUPLICATE release heading(s)");
expect(r.output).toContain("## 0.1.1");
// The old message must NOT be what this tree gets.
expect(r.output).not.toContain("nothing could have been deleted");
});
it("...and STRICT does not change that — it was never a skip", async () => {
const dup = `# Changelog\n\n## Unreleased\n\n${dated("0.1.1")}${body}\n${dated("0.1.1")}${body}`;
const r = await check(repoIntroducing(dup), {
CHANGELOG_MONOTONIC_STRICT: "1",
});
expect(r.code).toBe(1);
expect(r.output).toContain("DUPLICATE release heading(s)");
});
it("...while a CLEAN introduced changelog still passes, SAYING uniqueness ran", async () => {
const r = await check(repoIntroducing(BASE));
expect(r.code).toBe(0);
expect(r.output).toContain("nothing could have been deleted");
expect(r.output).toContain("uniqueness on HEAD already passed");
});
it("a duplicate OUTSIDE a git work tree is caught (#133)", async () => {
// No git at all — a tarball, an unpacked release. Uniqueness still has
// everything it needs; only containment does not.
const dir = mkdtempSync(join(tmpdir(), "cast-monotonic-nogit-"));
writeFileSync(
join(dir, "CHANGELOG.md"),
`# Changelog\n\n${dated("0.1.1")}${body}\n${dated("0.1.1")}${body}`,
);
const r = await run("bash", [MONOTONIC, "base"], {}, dir);
expect(r.code).toBe(1);
expect(r.output).toContain("DUPLICATE release heading(s)");
});
it("a duplicate is caught even when the base ref will not resolve (#133)", async () => {
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), {}, "origin/no-such-branch");
expect(r.code).toBe(1);
expect(r.output).toContain("DUPLICATE release heading(s)");
expect(r.output).not.toContain("containment SKIPPED");
});
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);
@ -541,11 +614,13 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
// 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 () => {
it("an unresolvable base ref SKIPS CONTAINMENT locally — not everything (#133)", 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");
expect(r.output).toContain("containment SKIPPED");
// ...and it must not claim nothing was checked: uniqueness already ran.
expect(r.output).toContain("already ran and passed");
expect(r.output).not.toContain("Nothing was checked");
});
it("...and the SAME condition is a hard FAILURE under STRICT=1, naming fetch-depth", async () => {
@ -558,16 +633,23 @@ describe("changelog-monotonic.sh — release headings are append-only (#133)", (
expect(r.output).toContain("CHANGELOG_MONOTONIC_STRICT=1");
expect(r.output).toContain("fetch-depth: 0");
expect(r.output).not.toContain("SKIPPED");
// Even here the message must scope itself to containment (#133).
expect(r.output).toContain("it is containment that cannot run");
});
// 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", () => {
it("ci.yml runs it on EVERY event, 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'");
// #133: NOT pull-request-only. Deletion is vacuous on a push to main, but
// duplication is vacuous on no tree — gating the whole script left a
// duplicate that reached main by any other route unasserted forever.
expect(CI).not.toContain("if: github.event_name == 'pull_request'");
// ...and dropping that gate is only safe WITH the fallback: on a push
// `github.base_ref` is empty, a bare `origin/` does not resolve, and
// STRICT promotes that to a hard failure on every push to main.
expect(CI).toContain('"origin/${{ github.base_ref || github.ref_name }}"');
expect(CI).toContain("CHANGELOG_MONOTONIC_STRICT");
// ...which is only reachable because the checkout has the base history.
expect(CI).toContain("fetch-depth: 0");