diff --git a/.github/scripts/changelog-monotonic.sh b/.github/scripts/changelog-monotonic.sh index 904efdf..999e74e 100755 --- a/.github/scripts/changelog-monotonic.sh +++ b/.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 " (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: '## ...' where 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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 998313d..6c6c9f9 100644 --- a/.github/workflows/ci.yml +++ b/.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 - # the same reason as the one above — when it goes red the log names the - # invariant that broke — but a DIFFERENT invariant: armed 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 + # ...and no SHIPPED release heading was deleted or DUPLICATED (#122, #143). + # Its own step for the same reason as the one above — when it goes red the + # log names the invariant that broke — but a DIFFERENT invariant: armed 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. - - 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e6644d..c82aac5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/test/release.sh b/test/release.sh index acb0f46..11c06c8 100644 --- a/test/release.sh +++ b/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() { # — 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"