forked from heavy-duty/box
fix(changelog-monotonic): check uniqueness before anything base-side
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. Deletion needs a diff to see; duplication is the one release-notes.sh actually mis-renders, re-arming its grab on the second heading (#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 reviewing heavy-duty/rig#99 and heavy-duty/cast#134, which inherited the ordering from here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
81c44c8b02
commit
94f6ed0047
4 changed files with 146 additions and 32 deletions
52
.github/scripts/changelog-monotonic.sh
vendored
52
.github/scripts/changelog-monotonic.sh
vendored
|
|
@ -59,26 +59,18 @@ skip() {
|
||||||
if [ "$strict" = "1" ]; then
|
if [ "$strict" = "1" ]; then
|
||||||
echo "changelog-monotonic: $* — and CHANGELOG_MONOTONIC_STRICT=1, so this is a FAILURE, not a skip." >&2
|
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 " 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
|
echo " Fix the checkout, not this script: the base ref must be fetched (fetch-depth: 0)." >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
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
|
exit 0
|
||||||
}
|
}
|
||||||
|
|
||||||
[ -f "$changelog" ] || { echo "changelog-monotonic: no such file: $changelog" >&2; exit 1; }
|
[ -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
|
# The set of RELEASE headings: '## <token> ...' where <token> looks like a
|
||||||
# version. Field $2, the same split changelog-armed.sh and release-notes.sh
|
# 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.
|
# use, so the three cannot disagree about what a section header is.
|
||||||
|
|
@ -90,14 +82,6 @@ headings_raw() {
|
||||||
}
|
}
|
||||||
headings() { headings_raw | sort -u; }
|
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) -------------------------------------
|
# --- uniqueness on HEAD (the #118 class) -------------------------------------
|
||||||
# Containment catches a DELETED heading. It cannot catch a DUPLICATED one: the
|
# 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
|
# duplicate is head-side SURPLUS, and `comm -23` (base minus head) is blind to
|
||||||
|
|
@ -149,6 +133,34 @@ EOF
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
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)"
|
base_headings="$(printf '%s\n' "$base_file" | headings)"
|
||||||
head_headings="$(headings < "$changelog")"
|
head_headings="$(headings < "$changelog")"
|
||||||
|
|
||||||
|
|
|
||||||
31
.github/workflows/ci.yml
vendored
31
.github/workflows/ci.yml
vendored
|
|
@ -68,19 +68,30 @@ jobs:
|
||||||
# says which check found the drift without anyone reading a suite.
|
# says which check found the drift without anyone reading a suite.
|
||||||
- name: changelog is armed for the next entry
|
- name: changelog is armed for the next entry
|
||||||
run: bash .github/scripts/changelog-armed.sh
|
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.
|
# 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:
|
env:
|
||||||
CHANGELOG_MONOTONIC_STRICT: '1'
|
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
|
# 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
|
# disposable VM, which is exactly the substrate the rehearsal needs. It runs
|
||||||
|
|
|
||||||
33
CHANGELOG.md
33
CHANGELOG.md
|
|
@ -97,6 +97,39 @@ which records not just what changed but what each drill run proved.
|
||||||
|
|
||||||
### Fixed
|
### 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. 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
|
- **A failed rollup read no longer reads as "nothing is failing"** — when
|
||||||
`gh pr view` returned nothing, the fallback left the `statusCheckRollup`
|
`gh pr view` returned nothing, the fallback left the `statusCheckRollup`
|
||||||
*key* absent, and `checks_state` collapsed that into the same `NONE` as a PR
|
*key* absent, and `checks_state` collapsed that into the same `NONE` as a PR
|
||||||
|
|
|
||||||
|
|
@ -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
|
# silently (which is the failure shape this repo keeps refusing). CI closes
|
||||||
# the hole from the other side with STRICT.
|
# the hole from the other side with STRICT.
|
||||||
G="$(grepo mono-nobase '## 0.8.0 — 2026-07-19' '' '- **Shipped**')"
|
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: ...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: ...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
|
check "monotonic: STRICT turns that skip into a red run" 1 "is a FAILURE, not a skip" mono_strict "$G" no-such-ref
|
||||||
|
|
@ -402,11 +403,60 @@ check "monotonic: ...and points at the checkout, not the script" 1 "fetch-depth:
|
||||||
# Outside a work tree at all (a tarball, an unpacked release).
|
# Outside a work tree at all (a tarball, an unpacked release).
|
||||||
mkdir -p "$WORK/mono-nogit"
|
mkdir -p "$WORK/mono-nogit"
|
||||||
printf '%s\n' '# Changelog' '' '## 0.8.0 — 2026-07-19' > "$WORK/mono-nogit/CHANGELOG.md"
|
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
|
mono "$WORK/mono-nogit" main
|
||||||
check "monotonic: a missing changelog refuses by path (never a skip)" 1 "no such file" \
|
check "monotonic: a missing changelog refuses by path (never a skip)" 1 "no such file" \
|
||||||
bash "$MONO" main "$WORK/nope.md"
|
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
|
||||||
|
|
||||||
# --- and the real tree, through the real script ----------------------------
|
# --- and the real tree, through the real script ----------------------------
|
||||||
# HEAD as its own base: the merge base is HEAD, so the sets are identical by
|
# 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
|
# construction. Proves the script runs against the actual CHANGELOG.md and
|
||||||
|
|
@ -424,6 +474,14 @@ check "ci.yml: ...with full history, or the merge base is unreachable" 0 "" \
|
||||||
grep -qF 'fetch-depth: 0' "$ROOT/.github/workflows/ci.yml"
|
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 "" \
|
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"
|
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.
|
||||||
|
check "ci.yml: the monotonic step is not gated to pull_request (#143)" 1 "" \
|
||||||
|
grep -qF "if: github.event_name == 'pull_request'" "$ROOT/.github/workflows/ci.yml"
|
||||||
|
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 "" \
|
check "CONTRIBUTING: names the append-only rule for release headings" 0 "" \
|
||||||
grep -qF 'changelog-monotonic.sh' "$ROOT/CONTRIBUTING.md"
|
grep -qF 'changelog-monotonic.sh' "$ROOT/CONTRIBUTING.md"
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue