heavy-duty/cast
7
0
Fork 0
Code Issues 4 Pull requests Projects Releases 3 Packages Wiki Activity Actions

fix(changelog-monotonic): check uniqueness before anything base-side

Browse source 
Uniqueness is a property of HEAD alone — no base ref, no merge base, no base
blob. It sat downstream of all three, so every degradation path returned
success on a tree carrying a duplicate.

The base-blob path was the worst: a branch that introduces CHANGELOG.md hit a
bare `exit 0` on a message that was true about deletion and silent about the
duplicate in front of it. STRICT could not reach it — STRICT guards the two
skip() calls, and that is not one of them.

That inverted the two halves, and it inverted them hardest here. Deletion
needs a diff to see; duplication is the one 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 (box#118). The half with the live extraction bug behind it
had the most ways to silently not run.

Moved, not rewritten. The skip messages now say containment skipped and that
uniqueness already passed. The CI step is no longer pull_request-only, with a
`github.ref_name` fallback because base_ref is empty on a push and a bare
`origin/` under STRICT would redden every push to main.

Found by claude-bot-andresmgsl and codex-bot-andresmgsl reviewing #134. cast
inherited the ordering from box, fixed there in heavy-duty/box#144 (#143).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-20 20:23:21 +00:00
parent 72030511b9
commit 0bd531042e
4 changed files with 190 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
.github/scripts/changelog-monotonic.sh vendored
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")"

35
.github/workflows/ci.yml vendored
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

40
CHANGELOG.md
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

102
test/release.test.ts
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");