fix(scripts): reject absorbed changelog release headings (#2264) - #2290
Merged
Conversation
…dings --check-bump now requires every bracketed release heading present at the fork point to still exist at head, catching absorbed predecessor sections that previously passed all three modes. Fixes #2264 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
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. |
Contributor
|
Claude finished @kyle-sexton's task in 38s —— View job Security review
Scope: Only the 2 files changed in this PR — no GitHub Actions workflow files are touched, so the Findings: none. Walked the new code paths specifically for shell/command injection and trust-boundary issues:
No secrets, tokens, or credentials are handled by this diff, and it doesn't touch CI trigger config or permissions. Nothing to flag. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 116b724746
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
changelog_bracket_headings must not treat ## [Unreleased] as a release heading; only bracketed SemVer versions count. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging. Re-run the job to retry the review; a new push also retries it only if the caller's Re-running does NOT help for every class:
|
…preserved-2264-63e1 Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-run the job to retry the review. A new push re-triggers this lane only if the caller's |
This was referenced Aug 12, 2026
kyle-sexton
added a commit
that referenced
this pull request
Aug 12, 2026
…line (#2327) (#2341) ## Summary #2290's absorbed-heading check diffs whole heading **lines**: `changelog_bracket_headings()` is `grep -E` without `-o`, so it emits the entire line, and `comm -23` compares those lines. Any edit to an existing release heading therefore reads as a deletion — including both annotations [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) itself prescribes, which **preserve** the heading they annotate. Measured against the shipped script on `main`, copied unmodified into a throwaway repo (#2327 carries the full reproduction): | change | heading at head | `--check-bump` on `main` | |---|---|---| | mark a pulled release | `## [1.0.0] - 2026-01-01 [YANKED]` | **EXIT=1** `ABSORBED CHANGELOG HEADING … ## [1.0.0]` | | date an existing release | `## [1.0.0] - 2026-01-01` | **EXIT=1** `ABSORBED CHANGELOG HEADING … ## [1.0.0]` | `1.0.0` is documented in both. Nothing was absorbed. This is a **required** merge check, so a false positive here blocks every in-flight PR that touches a heading line, not just its author's. ## Fix Key the comparison on the **version** each heading names, through the shared `changelog_versions` extractor, instead of on the rendered line. `## [1.0.0]` and `## [1.0.0] - 2014-12-13 [YANKED]` are one release, so annotating passes; renaming `## [0.51.8]` to `## [0.51.9]` still deletes the version `0.51.8`, so a relabel is still caught. `changelog_bracket_headings()` is dropped. It was a second reader of the same file, which the header's own `rendered_lines` note argues against — *"One tracker, not three, so the modes cannot drift."* The failure message still prints `## [<version>]`, so nothing downstream of it changes. **Two behaviour changes worth naming**, both consequences of keying on the version: - **Reformatting a heading between the bracketed and unbracketed forms is no longer a deletion.** The format of a release entry is `CHANGELOG FORMAT`'s concern, not preservation's. - **An unbracketed heading is now protected.** A bracketed-only matcher could not see `## 0.9.0 — 2025-12-01` at all, so absorbing that section was invisible. Both directions have a test. ## Deliberately out of scope All pre-existing from #2290 and tracked in #2327 — a false positive on a required gate is worth a surgical fix, not a redesign in the same change set: - The check lives inside `--check-bump`'s loop, so an absorption in a change set that does **not** bump the manifest is unpoliced (verified EXIT=0). - `docs/conventions/*/CHANGELOG.md` is outside `--check-bump`'s scope entirely, so an absorption there is unpoliced (verified EXIT=0). - The `2>/dev/null` process substitution on `git show` is fail-open: a git read that genuinely fails yields an empty fork-point set and reads as "nothing to preserve". #2327 sketches the fuller alternative (preservation as its own diff mode over every changed changelog under both roots), with a worked implementation on `fix/2264-changelog-preservation`. ## Test plan **Suite: `PASS=62 FAIL=0`** (57 pre-existing + 5 new), `bash scripts/check-changelog-parity.test.sh`. New cases: - `dating an existing heading is not an absorbed heading` — the FP above. - `marking a release '[YANKED]' preserves its heading and passes --check-bump` — the FP above. - `a relabelled predecessor heading still fails --check-bump` — the check must not go soft. - `reformatting a heading between the two accepted forms is not an absorbed heading`. - `absorbing an unbracketed release section is caught` — coverage a bracketed-only matcher could not have. `#2290`'s own absorption test is untouched and still passes, asserting on `## [0.51.8]` in the output. **The new tests are load-bearing** — established before the fix existed, by running `main`'s shipped script unmodified against the same two fixtures the new tests use (full transcript in #2327): | fixture | `main`'s script | this branch | |---|---|---| | `## [1.0.0]` → `## [1.0.0] - 2026-01-01 [YANKED]` | **EXIT=1** `ABSORBED CHANGELOG HEADING … ## [1.0.0]` | EXIT=0 | | `## [1.0.0]` → `## [1.0.0] - 2026-01-01` | **EXIT=1** `ABSORBED CHANGELOG HEADING … ## [1.0.0]` | EXIT=0 | The symmetric in-tree check — restoring only `scripts/check-changelog-parity.sh` from `origin/main` and re-running the suite — was **not** run: the command was refused by a local permission classifier, and it was not reshaped to get around it. The table above is the same evidence by a different route. **Zero false positives on real history.** `main` is squash-merged, so each commit on it *is* a merged PR. Every one of the 60 most recent commits touching `plugins/*/CHANGELOG.md` was replayed as its own PR (tree at `C`, base `C^1`) in a throwaway local clone, running this branch's `--check-bump`: | | | |---|---| | commits replayed | **59** (of 60; `cf743d61` could not be checked out) | | clean | **58** | | fired | **1** | | of which `ABSORBED CHANGELOG HEADING` | **0** | The single fire is `78dbb10e`, reporting `UNDOCUMENTED BUMP` and three `VERSION REGRESSION`s — none from the check this PR touches. **The zero is honest, and it is itself a finding.** The one commit in recent history that really does delete a release heading — `04822fc4`, which folded `docs-hygiene`'s never-released `## [0.9.7]` into `## [0.10.0]` — is in the replay window (verified) and is *not* caught, because it changed **no manifest at all** (`git diff --name-only 04822fc^1 04822fc` lists only `ci.yml`, that changelog, and the two gate files). Living inside `--check-bump`'s loop, the check never reaches a plugin whose version did not change. That is #2327 item 3, now demonstrated on real history rather than a synthetic fixture. This PR does not change that scoping — it fixes the false positives — so the sweep's role here is to show the fix **introduces no new fires** across 59 real commits, not to demonstrate detection. Detection is demonstrated by the unit cases. Replaying a squash commit against its parent is not identical to what CI saw at merge time — CI resolved the fork point of the original branch, which for a stale branch sat further back — so fires from `--check-bump`'s *other* checks (`VERSION REGRESSION`, `UNDOCUMENTED BUMP`) are artifacts of comparing against a parent the branch never saw, plus checks that did not exist when those commits merged. The **absorbed-heading** check is unaffected: `C^1` is an ancestor of `C`, so the fork point and the parent coincide and the comparison reads exactly the diff that commit introduced. **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)`. **Conflict-marker sweep**, both files: `<<<<<<<` 0, `|||||||` 0, `>>>>>>>` 0, `=======` 4 — all four pre-existing `# ==== <section> ====` banner comments in the test file, none introduced here (0 marker-shaped lines among this diff's additions). ## Related Closes #2327 — the live false positives it reports. Its remaining items (the scoping gaps and the fail-open fork-point read, all pre-existing from #2290 and untouched here) were split into **#2342** so this close is honest rather than partial. #2290 introduced the check being repaired; #2264 is the defect it closed. #2154 / #2159 are the SIGPIPE regression in this same gate — the reading discipline is untouched here, since neither `changelog_versions` nor `mapfile` exits before EOF. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 12, 2026
kyle-sexton
added a commit
that referenced
this pull request
Aug 12, 2026
…2370) Fixes #2342 ## Summary - Add `--check-preserved <ref>` mode that checks every **touched** changelog (plugin and convention) for dropped version headings vs the fork point. - Replace the in-loop absorbed-heading check inside `--check-bump` (which only ran for bumped plugins) with this dedicated mode. - Wire `--check-preserved` into the CI `changelog-parity-gate` job as an additional PR-only step. - Uses `git ls-tree` + command-substitution `git show` (fail-loud on git errors). ## Test plan - [x] `bash scripts/check-changelog-parity.test.sh` (76/0) - Covers: changelog-only absorption, convention changelogs, relabelling, [YANKED], heading reformat, stale-branch false-positive guard, whole-file deletion, SIGPIPE/gawk fixture ## Related - #2264 / #2290 — motivating absorption defect - #2327 / #2341 — false-positive fixes for heading annotations - #2324 — git fail-open hardening (overlapping fix, landed separately in PR #2369) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2264.
--check-bumpnow fails when a bumped plugin's CHANGELOG.md loses any## [x.y.z]heading that existed at the merge base (fork point). This catches merge-forward resolutions that fuse two releases into one section — the reproduction from the issue where## [0.51.8]vanishes while0.51.9documents the bump.Compares against the fork point, not
maintip, so a branch that bumps past an advanced base without yet carrying another branch's release heading is not a false positive.Verification
bash scripts/check-changelog-parity.test.sh— PASS=57 FAIL=0Related