changelog-monotonic: the uniqueness half is gated behind base-side conditions it does not need #143

New issue

Closed

opened 2026-07-20 20:13:04 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-20 20:13:04 +00:00

(Migrated from github.com)

changelog-monotonic.sh has two halves. Containment (no shipped heading was
deleted) is a property of a diff and genuinely needs the merge base.
Uniqueness (no version heading appears twice) is a property of HEAD
alone — it needs no base ref, no merge base, and no base blob.

But uniqueness sits downstream of all three. At
.github/scripts/changelog-monotonic.sh#L95-L99:

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 ... — nothing could have been deleted."
  exit 0
}

dupes= is at L118, after that. So a tree carrying a duplicate exits 0 on
a message that is true about deletion and silent about the duplicate sitting in
front of it.

Reproduced

A branch that introduces CHANGELOG.md (absent at the merge base) carrying two
## 0.8.0 headings, under STRICT=1:

changelog-monotonic: CHANGELOG.md does not exist at the merge base (ce6a18a) — nothing could have been deleted.
EXIT=0

The duplicate is right there — grep -c '^## 0.8.0' returns 2.

STRICT does not cover this path: it guards the two skip() calls, and this is
a plain exit 0. The skip() paths have the same shape for the same reason —
locally (STRICT unset) a shallow clone or a non-git tree returns 0 without ever
looking at a duplicate the author is about to push.

Credit: found by claude-bot-andresmgsl reviewing the ports in
heavy-duty/rig#99 and heavy-duty/cast#134, which inherited the ordering from
here.

Why it matters more than the exit code suggests

The duplicate half is not the redundant one. release-notes.sh re-arms its
grab on every matching ## line, so a duplicated heading makes the published
body absorb whatever sits between the copies — that is the box#118 shape, and
the reason the uniqueness assert was added alongside containment rather than
instead of it. It is also the half with the most ways to silently not run.

Fix

A move, not a rewrite: hoist headings_raw and the dupes block to directly
after the [ -f "$changelog" ] check, above git rev-parse --is-inside-work-tree. The skip messages then become accurate — they would be
skipping only containment, the half that actually needed the history.

Tests should pin the ordering, not just the outcome: the existing base-absent
case asserts exit 0 without asserting that uniqueness still ran. Add a case
where the PR introduces CHANGELOG.md carrying a duplicate — green today,
red once the block moves.

The trigger, which is the same gap one level up

ci.yml runs the guard if: github.event_name == 'pull_request'. That is well
argued for containment — on a push to main the merge base is HEAD, so the
assert is vacuous — but it is wrong for uniqueness, which is vacuous on no tree.
If a duplicate ever reaches main by a route other than a PR, nothing asserts it.

Dropping the if naively does not work, and it fails closed in the noisiest
way: on a push event github.base_ref is empty, so the argument becomes
origin/, which does not resolve, and STRICT=1 promotes that skip to a hard
failure. Verified:

changelog-monotonic: base ref 'origin/' does not resolve here ... — and
CHANGELOG_MONOTONIC_STRICT=1, so this is a FAILURE, not a skip.

So the trigger change needs a base-ref fallback:

- 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 || github.ref_name }}"

On a PR that is origin/<base> as before. On a push to main it is
origin/main, whose merge base with HEAD is HEAD or its parent — containment
passes vacuously, exactly as the if intended, while uniqueness now runs on
every push.

Scope

.github/scripts/changelog-monotonic.sh — hoist the uniqueness block
.github/workflows/ci.yml — drop the if, add the base-ref fallback
test/release.sh — a case that pins uniqueness running when the base blob is
absent

Should land in all three repos together — rig and cast are porting this script
now (heavy-duty/rig#99, heavy-duty/cast#134) and will carry the fix in those
PRs, so box is the one that needs its own.

`changelog-monotonic.sh` has two halves. **Containment** (no shipped heading was deleted) is a property of a *diff* and genuinely needs the merge base. **Uniqueness** (no version heading appears twice) is a property of **HEAD alone** — it needs no base ref, no merge base, and no base blob. But uniqueness sits downstream of all three. At [.github/scripts/changelog-monotonic.sh#L95-L99](.github/scripts/changelog-monotonic.sh#L95-L99): ```sh 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 ... — nothing could have been deleted." exit 0 } ``` `dupes=` is at L118, *after* that. So a tree carrying a duplicate exits **0** on a message that is true about deletion and silent about the duplicate sitting in front of it. ## Reproduced A branch that introduces `CHANGELOG.md` (absent at the merge base) carrying two `## 0.8.0` headings, under `STRICT=1`: ``` changelog-monotonic: CHANGELOG.md does not exist at the merge base (ce6a18a) — nothing could have been deleted. EXIT=0 ``` The duplicate is right there — `grep -c '^## 0.8.0'` returns 2. `STRICT` does not cover this path: it guards the two `skip()` calls, and this is a plain `exit 0`. The `skip()` paths have the same shape for the same reason — locally (STRICT unset) a shallow clone or a non-git tree returns 0 without ever looking at a duplicate the author is about to push. Credit: found by `claude-bot-andresmgsl` reviewing the ports in heavy-duty/rig#99 and heavy-duty/cast#134, which inherited the ordering from here. ## Why it matters more than the exit code suggests The duplicate half is not the redundant one. `release-notes.sh` re-arms its grab on every matching `## ` line, so a duplicated heading makes the published body absorb whatever sits between the copies — that is the box#118 shape, and the reason the uniqueness assert was added alongside containment rather than instead of it. It is also the half with the most ways to silently not run. ## Fix A move, not a rewrite: hoist `headings_raw` and the `dupes` block to directly after the `[ -f "$changelog" ]` check, above `git rev-parse --is-inside-work-tree`. The skip messages then become accurate — they would be skipping only containment, the half that actually needed the history. Tests should pin the ordering, not just the outcome: the existing base-absent case asserts `exit 0` without asserting that uniqueness still ran. Add a case where the PR *introduces* `CHANGELOG.md` carrying a duplicate — green today, red once the block moves. ## The trigger, which is the same gap one level up `ci.yml` runs the guard `if: github.event_name == 'pull_request'`. That is well argued for containment — on a push to main the merge base *is* HEAD, so the assert is vacuous — but it is wrong for uniqueness, which is vacuous on no tree. If a duplicate ever reaches main by a route other than a PR, nothing asserts it. Dropping the `if` naively does not work, and it fails **closed** in the noisiest way: on a `push` event `github.base_ref` is empty, so the argument becomes `origin/`, which does not resolve, and `STRICT=1` promotes that skip to a hard failure. Verified: ``` changelog-monotonic: base ref 'origin/' does not resolve here ... — and CHANGELOG_MONOTONIC_STRICT=1, so this is a FAILURE, not a skip. ``` So the trigger change needs a base-ref fallback: ```yaml - 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 || github.ref_name }}" ``` On a PR that is `origin/<base>` as before. On a push to main it is `origin/main`, whose merge base with HEAD is HEAD or its parent — containment passes vacuously, exactly as the `if` intended, while uniqueness now runs on every push. ## Scope - `.github/scripts/changelog-monotonic.sh` — hoist the uniqueness block - `.github/workflows/ci.yml` — drop the `if`, add the base-ref fallback - `test/release.sh` — a case that pins uniqueness running when the base blob is absent Should land in all three repos together — rig and cast are porting this script now (heavy-duty/rig#99, heavy-duty/cast#134) and will carry the fix in those PRs, so box is the one that needs its own.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#143

No description provided.