Merge pull request #144 from dan-claude-bot/fix/monotonic-uniqueness-first

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

.github/scripts/changelog-monotonic.sh

@@ -59,26 +59,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 "changelog-monotonic: containment SKIPPED — $*"
-  echo "  (Nothing was checked. In CI this same condition is a hard failure.)"
+  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 changelog-armed.sh and release-notes.sh
 # use, so the three cannot disagree about what a section header is.
@@ -90,14 +82,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 #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
@@ -149,6 +133,34 @@ 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 (#143). It asks nothing of the history, and gating it
+# behind these conditions let a duplicate exit 0 on a message about deletion.
+
+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")"
 
@@ -197,4 +209,17 @@ EOF
 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"
+head_count="$(printf '%s\n' "$head_headings" | grep -c . || true)"
+
+# The success line has two honest forms, because this step now runs on two
+# shapes of event. On a push to main the merge base IS HEAD: containment
+# compared the file against itself and asserted nothing, and deletion is
+# undetectable on that event by construction. Reporting "all N still present"
+# there would be the same dishonesty the skip messages were fixed for — a log
+# claiming a check that did no work. Uniqueness is the half that actually ran,
+# so that is the half the line names.
+if [ "$merge_base" = "$(git rev-parse HEAD)" ]; then
+  echo "changelog-monotonic: containment vacuous (the merge base IS HEAD, so nothing could have been deleted between them) — uniqueness on HEAD checked $head_count release heading(s)."
+else
+  echo "changelog-monotonic: all $count release heading(s) at the merge base ($(git rev-parse --short "$merge_base")) are still present in $changelog"
+fi

.github/workflows/ci.yml

@@ -68,19 +68,30 @@ jobs:
       # says which check found the drift without anyone reading a suite.
       - name: changelog is armed for the next entry
         run: bash .github/scripts/changelog-armed.sh
-      # ...and no SHIPPED release heading was deleted (#122). Its own step for
+      # ...and no SHIPPED release heading was deleted or DUPLICATED (#122, #143).
-      # the same reason as the one above — when it goes red the log names the
+      # Its own step for the same reason as the one above — when it goes red the
-      # invariant that broke — but a DIFFERENT invariant: armed is a fact
+      # log names the invariant that broke — but a DIFFERENT invariant: armed is
-      # about this tree, monotonicity is a fact about this tree versus its
+      # a fact about this tree, monotonicity is a fact about this tree versus
-      # merge base. Pull requests only: on a push to main the merge base IS
+      # its merge base. STRICT=1 so a checkout that cannot reach the base ref
-      # 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'
+      # NOT pull-request-only, and that is the #143 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 multi-user rehearsal, on a REAL incus — a GitHub runner is root on a
   # disposable VM, which is exactly the substrate the rehearsal needs. It runs

CHANGELOG.md

@@ -131,6 +131,46 @@ which records not just what changed but what each drill run proved.
 
 ### Fixed
 
+- **`changelog-monotonic.sh` no longer lets a duplicate heading through on the
+  paths where it cannot see the base** (#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.
+
+  This inverted the value of the two halves. Deletion is the failure that needs
+  a diff to see; duplication is the one `release-notes.sh` actually mis-renders,
+  re-arming its grab on the second heading and absorbing whatever sits between
+  the copies (#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 — and the success line got the same treatment, because dropping the
+  gate made `merge_base == HEAD` a routine path rather than a degradation. On a
+  push to main containment compares the file against itself and asserts nothing,
+  so the line now reports containment *vacuous* and names uniqueness as the half
+  that ran, instead of claiming N headings were verified present by a comparison
+  that could not have detected their absence.
+
+  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` reviewing the ports in heavy-duty/rig#99 and
+  heavy-duty/cast#134, which inherited the ordering from here.
+
 - **A failed rollup read no longer reads as "nothing is failing"** — when
   `gh pr view` returned nothing, the fallback left the `statusCheckRollup`
   *key* absent, and `checks_state` collapsed that into the same `NONE` as a PR

test/release.sh

@@ -394,7 +394,8 @@ check "monotonic: a base with no release headings passes (nothing to delete)" 0
 # silently (which is the failure shape this repo keeps refusing). CI closes
 # the hole from the other side with STRICT.
 G="$(grepo mono-nobase '## 0.8.0 — 2026-07-19' '' '- **Shipped**')"
-check "monotonic: an unresolvable base ref SKIPS, saying nothing was checked" 0 "SKIPPED" mono "$G" no-such-ref
+check "monotonic: an unresolvable base ref SKIPS containment" 0 "containment SKIPPED" mono "$G" no-such-ref
+check "monotonic: ...and says uniqueness already ran, not that nothing did" 0 "already ran" mono "$G" no-such-ref
 check "monotonic: ...naming the base ref it could not resolve" 0 "no-such-ref" mono "$G" no-such-ref
 check "monotonic: ...and warning that CI treats it as a failure" 0 "hard failure" mono "$G" no-such-ref
 check "monotonic: STRICT turns that skip into a red run" 1 "is a FAILURE, not a skip" mono_strict "$G" no-such-ref
@@ -402,17 +403,86 @@ check "monotonic: ...and points at the checkout, not the script" 1 "fetch-depth:
 # Outside a work tree at all (a tarball, an unpacked release).
 mkdir -p "$WORK/mono-nogit"
 printf '%s\n' '# Changelog' '' '## 0.8.0 — 2026-07-19' > "$WORK/mono-nogit/CHANGELOG.md"
-check "monotonic: outside a git work tree it skips rather than erroring" 0 "SKIPPED" \
+check "monotonic: outside a git work tree it skips containment, not everything" 0 "containment SKIPPED" \
   mono "$WORK/mono-nogit" main
 check "monotonic: a missing changelog refuses by path (never a skip)" 1 "no such file" \
   bash "$MONO" main "$WORK/nope.md"
 
+# --- #143: 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 #143 the duplicate check sat downstream of the base-ref, merge-base
+# and base-blob conditions, so each of the three degradation paths below exited
+# 0 on a tree with 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 below was green before and after).
+grepo_nocl() { # a repo whose main has NO changelog at all
+  local d="$WORK/$1"
+  mkdir -p "$d"
+  git -C "$d" init -q -b main
+  git -C "$d" config user.email test@example.invalid
+  git -C "$d" config user.name test
+  echo seed > "$d/README.md"
+  git -C "$d" add README.md
+  git -C "$d" commit -qm base
+  git -C "$d" checkout -q -b pr
+  echo "$d"
+}
+add_changelog() { # <dir> <lines...> — the PR introduces the file
+  local d="$1"; shift
+  { echo "# Changelog"; echo; printf '%s\n' "$@"; } > "$d/CHANGELOG.md"
+  git -C "$d" add CHANGELOG.md
+  git -C "$d" commit -qm head
+}
+
+# The changelog is absent at the merge base AND the PR introduces a duplicate.
+G="$(grepo_nocl mono-143-newdup)"
+add_changelog "$G" '## Unreleased' '' '## 0.8.0 — 2026-07-19' '' '- **a**' '' '## 0.8.0 — 2026-07-19' '' '- **stranded**'
+check "monotonic: a duplicate introduced where the base had no changelog is CAUGHT (#143)" 1 "DUPLICATE release heading" mono "$G" main
+check "monotonic: ...and STRICT does not change that (it was never a skip)" 1 "DUPLICATE release heading" mono_strict "$G" main
+# ...and the clean counterpart still exits 0, now saying uniqueness did run.
+G="$(grepo_nocl mono-143-newok)"
+add_changelog "$G" '## Unreleased' '' '## 0.8.0 — 2026-07-19' '' '- **a**'
+check "monotonic: ...while a CLEAN introduced changelog still passes" 0 "nothing could have been deleted" mono "$G" main
+check "monotonic: ...saying uniqueness was checked, not that nothing was" 0 "uniqueness on HEAD already passed" mono "$G" main
+
+# No git at all: uniqueness still has everything it needs.
+mkdir -p "$WORK/mono-143-nogit"
+printf '%s\n' '# Changelog' '' '## 0.8.0 — 2026-07-19' '' '## 0.8.0 — 2026-07-19' > "$WORK/mono-143-nogit/CHANGELOG.md"
+check "monotonic: a duplicate OUTSIDE a git work tree is caught (#143)" 1 "DUPLICATE release heading" \
+  mono "$WORK/mono-143-nogit" main
+
+# Unresolvable base ref: same — the skip is containment's, not the script's.
+G="$(grepo mono-143-nobase '## 0.8.0 — 2026-07-19' '' '- **Shipped**')"
+head_changelog "$G" '## 0.8.0 — 2026-07-19' '' '- **a**' '' '## 0.8.0 — 2026-07-19' '' '- **b**'
+check "monotonic: a duplicate is caught even when the base ref will not resolve (#143)" 1 "DUPLICATE release heading" mono "$G" no-such-ref
+
+# --- the push-to-main shape: containment vacuous, uniqueness real ----------
+# With the pull_request gate gone (#143), merge_base == HEAD is a ROUTINE path,
+# not a degradation. Containment compares the file against itself and asserts
+# nothing, so a line reading "all N still present" would claim a check that did
+# no work — the same dishonesty the skip messages were fixed for. The success
+# line therefore has two forms, and this pins which one each event gets.
+G="$(grepo mono-vacuous '## 0.8.0 — 2026-07-19' '' '- **Shipped**')"
+check "monotonic: HEAD as its own base reports containment VACUOUS, not verified" 0 "containment vacuous" mono "$G" HEAD
+check "monotonic: ...and names uniqueness as the half that actually ran" 0 "uniqueness on HEAD checked" mono "$G" HEAD
+# shellcheck disable=SC2016  # $1/$2 expand in the child shell, by design
+check "monotonic: ...and does NOT claim headings were still present" 1 "" \
+  bash -c 'cd "$1" && bash "$2" HEAD | grep -q "are still present"' _ "$G" "$MONO"
+# The PR shape keeps the containment wording — the two must not collapse.
+head_changelog "$G" '## 0.8.0 — 2026-07-19' '' '- **Shipped**' '' '## 0.9.0 — 2026-07-20' '' '- **New**'
+check "monotonic: a real base still reports containment, naming the count" 0 "still present" mono "$G" main
+
 # --- and the real tree, through the real script ----------------------------
 # HEAD as its own base: the merge base is HEAD, so the sets are identical by
 # construction. Proves the script runs against the actual CHANGELOG.md and
 # parses its real headings, without depending on an `origin/main` that a
 # fresh clone or a detached CI checkout may not have.
-check "monotonic: THIS tree passes against itself (the parser meets reality)" 0 "still present" \
+# HEAD as its own base is now the VACUOUS-containment path (#143), so the
+# assertion moved to uniqueness's count — which is the stronger proof of the
+# original intent anyway: it says the parser read the REAL CHANGELOG.md and
+# found real headings in it, rather than that a self-comparison came out equal.
+check "monotonic: THIS tree passes against itself (the parser meets reality)" 0 "uniqueness on HEAD checked" \
   mono "$ROOT" HEAD
 
 # The guard is only a guard if CI runs it — and only if CI runs it with the
@@ -424,6 +494,30 @@ check "ci.yml: ...with full history, or the merge base is unreachable" 0 "" \
   grep -qF 'fetch-depth: 0' "$ROOT/.github/workflows/ci.yml"
 check "ci.yml: ...and STRICT, so a skip is a red run and not a green one" 0 "" \
   grep -qF 'CHANGELOG_MONOTONIC_STRICT' "$ROOT/.github/workflows/ci.yml"
+# #143: the step must NOT be pull-request-only — duplication is vacuous on no
+# tree — and dropping that gate is only safe with the base-ref fallback, since
+# `github.base_ref` is empty on a push and a bare `origin/` under STRICT is a
+# hard failure on every push to main.
+# Scoped to the step's OWN block, deliberately. A file-wide negative would
+# forbid any FUTURE step in ci.yml from being pull_request-gated and would fail
+# citing #143 when one legitimately is — #143 constrains this step, not the file.
+# Terminates on a new STEP or a new JOB. The job boundary is not optional: the
+# monotonic step is the LAST step of `check`, so stopping only at the next
+# `- name:` runs the block into the `rehearsal` job below and swallows its
+# job-level `if:`. That reintroduces the very bug the scoping fixed — an
+# unrelated edit failing while citing #143 — just moved from "any step in the
+# file" to "this step plus the head of the next job".
+mono_step_block() {
+  awk '/^      - name: no shipped changelog heading/ {f=1; print; next}
+       f && (/^      - / || /^  [^ ]/) {exit}
+       f {print}' "$ROOT/.github/workflows/ci.yml"
+}
+# Anchored: an `if:` appearing inside a `run:` line is not a step condition.
+mono_step_gated() { mono_step_block | grep -q '^        if:'; }
+check "ci.yml: the monotonic step itself is not pull_request-gated (#143)" 1 "" mono_step_gated
+check "ci.yml: ...and the block was actually found (guards the awk above)" 0 "changelog-monotonic" mono_step_block
+check "ci.yml: ...and falls back to ref_name, so a push has a base to resolve" 0 "" \
+  grep -qF 'github.base_ref || github.ref_name' "$ROOT/.github/workflows/ci.yml"
 check "CONTRIBUTING: names the append-only rule for release headings" 0 "" \
   grep -qF 'changelog-monotonic.sh' "$ROOT/CONTRIBUTING.md"