fix(scripts): gate changelog sections a change set deletes (#2264)

[kyle-sexton]

Summary

scripts/check-changelog-parity.sh policed the bump and never the preservation. A change set could delete an already-released section's heading — absorbing its notes into the new release — and all three modes passed. Reproduced verbatim from #2264, now a test fixture:

tree	--check-bump	--check-order	--check	--check-preserved
base ## [0.51.8], ## [0.51.7] → head ## [0.51.9], ## [0.51.7]	PASS	PASS	PASS	FAIL

Why each of the three missed it: --check-bump asks only about the bumped version and never enumerates the base's other headings; --check-order reads 0.51.9, 0.51.7 as correctly ordered with no duplicates, and a gap is not an ordering violation; --check compares the manifest against the changelog maximum. Git leaves no conflict marker behind for that resolution either — when two change sets bump one plugin, the conflict region tends to open below the newest heading, so a plausible merge-forward writes two releases under one heading silently. It happened on this repo during the #2163/#2171 drain and was caught only because a human ran a heading-list diff by hand.

This adds a fourth mode, --check-preserved <base-ref>: every ## [<version>] heading a touched changelog carried at the fork point must still be there at head. Wired as a PR-only step in the existing changelog-parity-gate job, so no required-context name changes.

Three design decisions

1. Its own mode, not an addition to --check-bump. --check-bump already resolves the base and the plugin scope, so folding it in is cheaper — but its scope is plugins whose manifest version changed, which is exactly where an absorbed heading would not be looked for. A section can be absorbed by a change set that never touches a manifest, and the convention changelogs under docs/conventions/*/ are unversioned by any manifest so --check and --check-bump never read them at all. The new mode sweeps every changed changelog under both roots --check-order reads. The two modes share one prologue for the base ref, the fork point, and the touched-path scan, so they cannot drift into reading the diff differently.

2. Fork point, never the base tip. The issue suggested git show "$base:$changelog". Compared against the base tip, a branch that has not integrated main reads every heading main added after the fork as "deleted" — a false positive on a required gate that would block every in-flight PR. The two semantics coincide in the scenario this catches, because absorbing a section requires having merged main forward (that is what opens the conflict region), and once the base is an ancestor of head the fork point is the base tip. Merge-base loses nothing and cannot produce that false-positive class at all. Guarded by its own test (a stale branch that never integrated main is not flagged), which fails under base-tip semantics.

3. No exemption list — decided against real evidence, not in the abstract. Two removals an author might reach for, and neither needs a deletion:

• A yanked release: Keep a Changelog keeps the heading and marks it ## [0.0.5] - 2014-12-13 [YANKED] — "The [YANKED] tag is loud for a reason" — so the canonical "legitimate deletion" is not a deletion at all under the convention these changelogs follow. A [YANKED]-marked heading keeps its version in the extractor's list and passes; there is a test for it.
• A note written against a version the manifest then skipped: keep the heading and say so in the section body. The remedy --check suggests first — bump the manifest onto that number — is often unavailable, because by the time the mismatch is noticed the manifest has already moved past it and bumping back down is a VERSION REGRESSION under --check-bump.

The sweep below found exactly one deletion in this repo's recent history — 04822fc, which closed #2131 by folding docs-hygiene's never-released ## [0.9.7] into ## [0.10.0] — and it is the second shape. That is the strongest available argument for a hatch, and it is also the argument against one: in the diff, that fold is indistinguishable from the absorption this gate exists to catch. A structural exemption would therefore have to exempt both. An allowlist file (the repo's changelog-parity-baseline.txt / contract-slice-baseline.txt idiom) was the alternative considered and declined: it cannot be stale-guarded meaningfully here (post-merge the removal is in the base, so an entry never expires without red-lining main for someone else), and a required gate whose whole purpose is catching a hurried merge resolution should not ship the off switch a hurried resolver will reach for. The failure message states the annotate-in-place remedy instead.

Renames. The same bad resolution can relabel ## [0.51.8] to ## [0.51.9] rather than delete it. That is a deletion of 0.51.8 and the check reports it as one — correctly — and the failure message names relabelling explicitly so the author recognises what their resolve did. Separately, a plugin directory rename can carry an absorption past the gate (default rename detection makes git diff --name-only report only the new path); that is recorded in the code as a known boundary, not an oversight — --check-bump's manifest scoping has the same property, and --no-renames would not close it.

Constraints of the file that were preserved

• SIGPIPE (fix(ci): close the reverse direction of the changelog-parity gate #2154 / fix(ci): stop has_heading's early exit from SIGPIPE-failing the parity gate #2159). No reader in the new path exits before EOF — the heading lists go through the same rendered_lines writer into grep -oE and mapfile, none of which stop early — so no writer can take SIGPIPE under pipefail on any awk. Covered by a large-changelog fixture under a forced-gawk PATH shim, the same idiom the --check-bump SIGPIPE fixture uses. CI confirms it ran rather than skipped (no SKIP: line in the job log).
• Fixed-string, column-one heading matching and SemVer metacharacter escaping are untouched; the new mode reuses the shared changelog_versions extractor rather than adding a second reader, so a fenced or HTML-commented example heading is invisible to it too.
• Fail-closed git reads. Existence at the fork point is probed with git ls-tree (missing path → exit 0, empty output; unusable rev → non-zero), not git cat-file -e, which exits 128 for both and would have failed every change set that adds a changelog. Verified empirically before relying on it. The heading extraction's status is deliberately ignored — grep -oE exits 1 on a changelog with no version headings, which is not an error, and treating it as one would false-positive on every prose-only changelog (covered by a test).
• Not a single file under plugins/** is touched.

Test plan

Suite: PASS=76 FAIL=0 (55 pre-existing + 21 new), bash scripts/check-changelog-parity.test.sh, and green on CI (ubuntu-24.04).

New cases prove the check discriminates, not merely that it exists:

• FAILS on the issue's absorption shape, naming the vanished version — and the same test asserts all three other modes stay green on that very tree, so a future edit cannot quietly make this mode redundant without the suite noticing.
• FAILS on a relabelled heading, and the message names relabelling as a cause.
• PASSES on a legitimate bump that adds a heading and preserves its predecessors.
• PASSES on: a stale branch that never integrated main (the false-positive guard), a changelog added by the change set, a plugin removed outright, a [YANKED]-marked heading, a heading reformatted between the two accepted forms, a headingless changelog, a removed fenced example.
• FAILS on a changelog deleted while its plugin survives — and gets the restore-the-file remediation rather than the absorbed-heading one.
• Convention changelogs (docs/conventions/*/CHANGELOG.md) are in scope, proven by an absorption there.
• Large-changelog SIGPIPE fixture under forced gawk; fail-loud (exit 2) on a diff that cannot be computed; usage errors → exit 2.

Zero false positives on the live tree

Corpus: 65 plugin changelogs + 10 convention changelogs = 75 in scope, carrying 1530 version headings today. A separate scan confirms 0 of the 1487 headings in the plugin changelogs currently name a version above their manifest (the remaining 43 headings are in the 10 convention changelogs, which no manifest versions, so there is nothing to compare them against).

Sweep B — the load-bearing one. main is squash-merged (git rev-list --merges origin/main → 0 results), so each commit on main is a merged PR. Every one of the 60 most recent commits touching a changelog was replayed as its own PR (tree at C, base C^1) in a throwaway local clone:

commits replayed	60
changelogs entering scope (summed)	243
headings compared (summed)	7789
clean	59
fired	1

The single fire is 04822fc — plugins/docs-hygiene/CHANGELOG.md documented 0.9.7 at the fork point but no longer does — and it is a true positive: that commit really did delete ## [0.9.7] and move its bullet up under ## [0.10.0] (diff). It was a legitimate fold of a never-released entry, which is why design decision 3 above is written the way it is. A sweep that never fires across 60 real commits would be weaker evidence than one that fires exactly on the commit that deleted a heading.

Sweep A — whole-corpus, older bases. The real mode run from a tree at main with progressively older bases, so every heading present at that base must survive at head:

base	changelogs in scope	headings compared	result
origin/main~10	19	671	rc=0
origin/main~50	44	1252	rc=0
origin/main~200	67	1108	rc=0
origin/main~600	74	609	rc=0

(A root-commit base was also run; it compares 0 headings because no changelog existed then, so it is not evidence and is excluded from the table.)

Conflict-marker sweep. <<<<<<<: 0. |||||||: 0. >>>>>>>: 0. =======: 8 matches, every one inside a # ===== <section> ===== banner comment matching the file's existing section-header style — none a conflict marker.

Other gates, locally: shellcheck --rcfile=.shellcheckrc -x on both files → clean; scripts/check-shell-portability.sh --paths on both files → No unexcused GNU-only constructs in 2 shell file(s); scripts/check-silent-skips.sh --check → clean; actionlint .github/workflows/ci.yml → clean.

Independent verification. A fresh-context verifier reads the diff at the pinned head SHA, builds its own sweep (not the author's scripts), reverts the fix to prove the new tests fail without it, and confirms the SIGPIPE-safe reading discipline. Its verdict is posted to this PR as a comment.

Related

Closes #2264.

Same file, different failure: #2154 / #2159 (the SIGPIPE regression that made a present heading read as missing; this one made a missing heading read as fine). #2131 is the reverse-parity gap whose remediation commit is the single true-positive fire in Sweep B. The incident that produced the live absorption: #2163 / #2171.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 2m 17s —— View job

---

Security review complete

• Gather context (diff, changed files)
• Review .github/workflows/ci.yml changes for permission-widening / dangerous trigger patterns
• Review scripts/check-changelog-parity.sh for injection / unsafe git-ref handling / logic flaws
• Review scripts/check-changelog-parity.test.sh for test-harness security issues
• Post findings

No security issues found. This PR adds a new --check-preserved mode and a new PR-only CI step; scope was limited to the three changed files (gh pr diff against origin/main).

Notes from the review (informational, not findings):

• CI trigger/permissions: the workflow stays on pull_request (not pull_request_target/workflow_run), top-level permissions: contents: read is unchanged, the new changelog-parity-gate step adds no job-level permissions: override, and actions/checkout remains SHA-pinned with persist-credentials: false — ci.yml#L527-L535.
• github context handling: the new step passes github.base_ref through an env: var (BASE_REF) rather than interpolating ${{ github.base_ref }} directly into the run: script — the same indirection pattern already used by the adjacent --check-bump step, which avoids script injection via the github context.
• Git argument handling in the new --check-preserved code path (check-changelog-parity.sh#L435-L494): $changelog is always a path sourced from git diff --name-only/glob matching against plugins/*/CHANGELOG.md or docs/conventions/*/CHANGELOG.md, and every git invocation that takes it either uses a -- path separator (git ls-tree -r --name-only "$merge_base" -- "$changelog") or embeds it as a rev:path argument (git show "$merge_base:$changelog") rather than a bare leading argument, so there's no option-injection surface even for an attacker-controlled filename in their own PR diff. $base/$merge_base are resolved via git rev-parse/git merge-base (always commit SHAs), not passed through unvalidated.
• Fail-closed discipline preserved: the new mode reuses the shared prologue's command-substitution-with-status-check pattern (not process substitution) for git ls-tree and git show, so a git read that genuinely fails exits 2 rather than silently passing — consistent with the existing --check-bump discipline.
• Per the review instructions, supply-chain/unpinned-action risk, dangerous trigger patterns, excessive permissions, and template injection are zizmor's advisory lane and were not re-litigated here; nothing in this diff touches those dimensions regardless (no new actions, no permission changes, no new triggers).
  · Branch: fix/2264-changelog-preservation

[claude]

Claude finished @kyle-sexton's task in 6m 0s —— View job

---

Review complete

• Gather context (diff, CLAUDE.md, REVIEW.md)
• Review scripts/check-changelog-parity.sh changes
• Review scripts/check-changelog-parity.test.sh changes
• Review .github/workflows/ci.yml changes
• Check test coverage claims against actual test run
• Post findings

Scope note: this repo has a claude-security-review.yml workflow, so per REVIEW.md's lane split I stuck to correctness/design/tests/docs and left security to that lane.

Execution note: the sandbox for this review blocked running shell commands beyond git/gh read operations (bash scripts/check-changelog-parity.test.sh, git init, git --version etc. all required approval I couldn't grant). I traced the new --check-preserved logic and every new test case by hand against the script instead of executing them. The PR body's PASS=75 FAIL=0, live-tree sweep, shellcheck/actionlint/portability results, and the Sweep B replay are author-claimed and unverified by me.

What I checked

Traced --check-preserved (scripts/check-changelog-parity.sh:435-494) end to end against each of the 20 new test cases: absorbed-predecessor, relabelled-heading, legitimate bump, stale-branch-never-integrated (fork point vs. base tip), newly-added changelog, plugin-removed, whole-file-deleted, [YANKED], reformatted heading, headingless file, fenced example, convention changelogs, SIGPIPE-safety (no reader in the new path exits before EOF — confirmed by reading, not running), and the no-common-ancestor/usage-error exits. The logic holds up in every case I traced, including the trickier ones:

• git ls-tree (not git cat-file -e) correctly distinguishes "path missing at fork point" (exit 0, empty) from "unusable rev" (non-zero) — verified this matters, since cat-file -e would exit 128 for both and break the "changelog added by this PR" case.
• The fork-point-not-base-tip choice is correct: merge-base is computed once in the shared prologue and reused, so it can't drift from what --check-bump uses.
• The [[ -f "$changelog" ]] / [[ ! -d "${changelog%/CHANGELOG.md}" ]] branch correctly separates "whole plugin directory removed" (skip, not a violation) from "changelog alone deleted, plugin survives" (flagged).
• The new CI step (.github/workflows/ci.yml:534-538) is correctly nested inside the existing changelog-parity-gate job alongside --check-bump, so no required-status-check context name changes — confirmed by reading the job boundaries, not just trusting the PR description.

One documented, intentional gap I confirmed rather than flagged: a plugin directory rename (not just a heading rename) can slip past --check-preserved, because default git rename detection (diff.renames, on since Git 2.9) makes git diff --name-only report only the new path when similarity is high, so the old path never enters touched_changelogs and the new path reads as "new at the fork point." The code comment at scripts/check-changelog-parity.sh:461-464 already states this tradeoff explicitly, so it's not a missed case, just worth knowing it's there.

Finding posted inline

🟡 Suggestion — scripts/check-changelog-parity.sh:482: the "DELETED CHANGELOG ENTRY" remediation text is written for the absorb/relabel case ("restore the heading above the entry that replaced it") but the same message also fires when the whole changelog file was deleted while its plugin survived — where that specific advice doesn't apply. Suggested a two-branch message; not a correctness bug in the check itself. See the inline comment for a concrete patch.

No Important/Critical findings. The design decisions in the PR description (fork point over base tip, no exemption list, shared prologue with --check-bump) all check out against the code as written.
· Branch: fix/2264-changelog-preservation

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[kyle-sexton]

Closing as duplicate of #2290, which merged the #2264 changelog-preservation fix via --check-bump fork-point heading checks.

[kyle-sexton]

Superseded by #2290, which landed the #2264 fix first — no objection to the close. Recording the delta here so the branch is not lost, and so the parts of it that are still live are traceable.

Filed as #2327: the shipped check diffs whole heading lines (changelog_bracket_headings uses grep -E without -o), so any edit to an existing release heading reads as a deletion. Both of these red-line the required gate today, verified against origin/main with the shipped script unmodified:

• ## [1.0.0] → ## [1.0.0] - 2026-01-01 [YANKED] — the canonical Keep a Changelog yank treatment, which preserves the heading — EXIT=1.
• ## [1.0.0] → ## [1.0.0] - 2026-01-01 — adding a release date — EXIT=1.

Plus two scope gaps the #2264 body had named as reasons to consider a separate mode: an absorption in a change set that does not bump the manifest, and one in a docs/conventions/*/CHANGELOG.md, both EXIT=0.

The branch fix/2264-changelog-preservation (head dbfa90e1) carries a worked alternative — preservation as its own diff mode over every changed changelog under both roots, keyed on the extracted version rather than the rendered line, so a dated or [YANKED]-marked heading reads as preserved while a relabel still reads as the deletion it is. 21 tests, swept clean across 60 replayed main commits (7789 headings compared) with one true-positive fire on 04822fc4. If #2327 is taken up, it needs rebasing onto #2290 and must replace the in-loop check rather than sit beside it.

This PR is not reopenable (the head ref moved while it was closed), so any re-land is a fresh PR from the same branch.