fix(scripts): add --check-preserved for changelog heading deletions

[kyle-sexton]

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

• 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

• check-changelog-parity: a merge-forward that absorbs the predecessor release section passes all three modes — deleted headings are unpoliced #2264 / fix(scripts): reject absorbed changelog release headings (#2264) #2290 — motivating absorption defect
• check-changelog-parity: the #2290 absorption check false-positives on any heading-line edit ([YANKED], dates), and leaves the no-bump and convention-changelog surfaces unpoliced #2327 / fix(scripts): key the absorbed-heading check on the version, not the line (#2327) #2341 — false-positive fixes for heading annotations
• fix(scripts): the absorbed-heading check fails OPEN on a git error, and misses unbumped plugins and convention changelogs #2324 — git fail-open hardening (overlapping fix, landed separately in PR fix(scripts): fail loud when absorbed-heading fork-point read errors #2369)

[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 6s —— View job

---

Security Review

• Fetch PR diff and read changed files
• Review .github/workflows/ci.yml changes for dangerous Actions patterns
• Review scripts/check-changelog-parity.sh for injection / trust-boundary issues
• Review scripts/check-changelog-parity.test.sh for test-harness security issues
• Post findings

Scope: commit 6ac9cd9, files .github/workflows/ci.yml, scripts/check-changelog-parity.sh, scripts/check-changelog-parity.test.sh.

Result: no exploitable vulnerabilities found

• Trigger/permissions: the workflow stays on pull_request (not pull_request_target/workflow_run) with top-level permissions: contents: read and persist-credentials: false on checkout — unchanged by this PR, no widening.
• Script injection via github context: the new step passes github.base_ref through an env: var (BASE_REF) and references it in run: only as a shell variable ("origin/$BASE_REF"), never interpolated directly into the script text — ci.yml#L527-L535. This is the safe pattern and matches the pre-existing --check-bump step.
• Command injection in the new --check-preserved mode: all attacker-influenced values (changelog paths from git diff --name-only, $merge_base) are passed as quoted arguments to git ls-tree / git show via command substitution, never through eval or unquoted expansion — check-changelog-parity.sh#L437-L452. No shell-metacharacter escape is possible even with adversarial file/branch names.
• Fail-open risk: both new git reads (git ls-tree, git show) check exit status and exit 2 on failure with an explicit "refusing to pass without checking" message, so a git error can't silently pass the gate — consistent with the existing fail-loud discipline in this file.
• Supply chain / action pins: no action references were added or re-pinned in this diff (only a job block removed/reordered); deferring to zizmor for pin-format checks per this lane's charter.

One item worth a human sanity-check (not a vulnerability)

The diff also fully removes the unrelated discriminating-test-skip-gate job (and its entry in needs: for the merge gate) — ci.yml#L456-L468 (job body deleted at the old location). That job is a self-test-integrity gate (scripts/check-discriminating-test-skips.sh) unrelated to changelog parity and not mentioned anywhere in this PR's description. It's not a permission-widening or injection issue, but silently dropping a required CI gate as an apparent side effect of an unrelated change is worth confirming was intentional (e.g., a stale rebase artifact) before merge, since its absence would go unnoticed otherwise.
· branch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ac9cd93c1

ℹ️ 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".

[claude]

Claude finished @kyle-sexton's task in 4m 51s —— View job

---

Review complete

• Gather context (CLAUDE.md, REVIEW.md, PR diff)
• Understand the request
• Review scripts/check-changelog-parity.sh
• Review scripts/check-changelog-parity.test.sh
• Review .github/workflows/ci.yml
• Post findings

Summary

The core feature — --check-preserved — is well designed: it correctly diffs against the fork point (not the base tip, avoiding false positives on branches that haven't merged main forward), keys comparisons on the extracted version rather than the rendered heading line (so dating/[YANKED]/reformatting annotations don't misread as deletions), fails loud (exit 2) on any git read it can't verify rather than silently passing, and correctly distinguishes "changelog added by this change set" (git ls-tree, not git cat-file -e, to avoid the exit-128-for-both ambiguity) from "changelog genuinely lost a heading." The whole-plugin-removal vs. changelog-only-deletion split, and the directory-rename boundary, are both handled and explicitly documented as known, reviewed limitations rather than oversights. I read through the full script and traced the logic by hand; I was not able to execute the test suite myself in this sandbox (bash scripts/check-changelog-parity.test.sh required approval I don't have here), so the 76/0 pass claim in the Test plan is author-claimed and not independently verified — but I did confirm the new --check-preserved test cases exercise the documented edge cases (absorption, relabelling, YANKED, reformatting, stale-branch, whole-file deletion, headingless files, fenced examples, convention changelogs, SIGPIPE/gawk) and that the assertions in each match the script's actual message text.

Two findings, both filed as inline comments, that look unrelated to this PR's stated purpose and likely accidental (possibly a stale branch state at commit time):

1. .github/workflows/ci.yml — this PR deletes the entire discriminating-test-skip-gate job and its needs: entry in ci-status, even though scripts/check-discriminating-test-skips.sh and its test file are completely untouched by this PR and still exist in the repo. Nothing in the PR description mentions retiring this gate. As written this silently disables a required CI check.
2. scripts/check-changelog-parity.test.sh — reverts away from the shared scripts/test-git-helpers.sh helpers that this branch's own fork point (origin/main, commit eb9f338) already uses, and that 3 other test files in scripts/ still use. The file now duplicates git -C "$repo" add -A && git -C "$repo" commit -qm ... at ~70 call sites and drops the git_init_safe protection against initializing a throwaway repo inside the real checkout.

No security-relevant findings in scope for this lane (this repo has a dedicated claude-security-review workflow, so injection/credential/auth findings are left to that lane — none of the new code here touches those surfaces regardless).
· Branch

[github-actions]

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