Skip to content

fix(scripts): the absorbed-heading check fails OPEN on a git error, and misses unbumped plugins and convention changelogs #2324

Description

@kyle-sexton

PR #2290 closed #2264 by adding an ABSORBED CHANGELOG HEADING check to --check-bump. The
motivating case is genuinely caught
— verified below as a control, so this is a hardening issue,
not a regression report.

Three residual holes, all reproduced against the real script copied unmodified out of origin/main.

Probe results

CONTROL   (bump absorbs 0.51.8)                     rc=1 :: ABSORBED CHANGELOG HEADING: alpha lost release section heading(s) ...
GAP 1     (heading dropped, NO manifest bump)       rc=0 :: Every plugin whose version changed ... has a '## [<version>]' entry.
GAP 2     (CONVENTION heading dropped)              rc=0 :: Every plugin whose version changed ... has a '## [<version>]' entry.
FAIL-OPEN (same absorb + git show CHANGELOG fails)  rc=0 :: Every plugin whose version changed ... has a '## [<version>]' entry.

1. Fail-open: process substitution discards git show's exit status

scripts/check-changelog-parity.sh at HEAD:

missing_headings="$(
  comm -23 \
    <(git show "$merge_base:$changelog" 2>/dev/null | changelog_bracket_headings - | sort) \
    <(changelog_bracket_headings "$changelog" | sort)
)"

<(git show …) is process substitution, so a git show failure is invisible: the base heading
list arrives empty, comm -23 reports nothing missing, and the gate passes without having checked.
2>/dev/null additionally hides the reason.

This is the exact pattern the same file's own header forbids, in prose, for this reason — the
diff_paths block a few lines above reads:

Read the change set's touched paths via COMMAND substitution, not process substitution: this gate
is a required CI merge check, and a git failure here must fail loud, never silently pass. Process
substitution swallows git's exit status …

The argument was already written down; the new code does the thing it argues against.

Reproduced by fault injection, not by inspection. A git shim on PATH fails git show only
for a path ending in CHANGELOG.md, so manifest resolution and every existing bump check still work
and only the preservation read breaks. The identical change set that the control catches (rc=1)
then passes:

# $D/bin/git
if [[ "${1:-}" == show && "${2:-}" == *CHANGELOG.md ]]; then
  echo "simulated object-store failure" >&2
  exit 128
fi
exec /usr/bin/git "$@"
FAIL-OPEN (same absorb + git show CHANGELOG fails)  rc=0

Fix: read the base blob with command substitution (or into a temp file) and check the status.
"Changelog absent at the merge base" must stay distinguishable from "git failed" — probe with
git cat-file -e "$merge_base:$changelog" first, since a changelog that is simply new in the
change set is a legitimate pass, and conflating the two either red-lines every new plugin or
restores the fail-open.

2. Scope: the check only runs for plugins whose manifest changed

It sits inside the per-manifest loop guarded by [[ -n "${bumped_candidate[$name]:-}" ]], so a
change set that edits a CHANGELOG and deletes a heading without touching plugin.json is never
examined. That is the quieter hiding place: the absorbed-heading resolution does not require a bump
to occur in the same commit.

GAP 1 (heading dropped, NO manifest bump)  rc=0

--check-bump already computes merge_base and diff_paths, so scoping preservation to the
changelogs the change set touched — rather than to bumped manifests — is a small change that
covers strictly more while still never red-lining a branch for a heading some earlier change
removed.

3. Scope: convention changelogs are entirely uncovered

changelog_bracket_headings matches only ^## \[x.y.z\], and the manifest loop only ever visits
plugins/*/. docs/conventions/*/CHANGELOG.md uses the ## 1.2.3 — date form and is unversioned by
any manifest, so it is invisible on both counts — even though --check-order explicitly scopes those
files and the file's own comment says convention changelogs are in scope "precisely because that is
where it shipped" (the loop-lane 6.0.0 → 3.1.1 → 5.0.0 → 4.0.0 incident).

GAP 2 (CONVENTION heading dropped)  rc=0

The file already has changelog_versions(), which goes through rendered_lines and matches every
heading form the repo uses — plugin, convention, and two-component. Using it for both sides would
close this and keep one reader across all modes, which is the stated design intent
("One tracker, not three, so the modes cannot drift").

Note on comm

comm requires sorted input and PR #2290 does sort both sides, so it is correct as written. Flagged
only so a later reader does not "simplify" the sort away — dropping it would silently produce wrong
answers while still exiting 0.

Reproduction

The probe script builds four disposable git repos, copies the real script in unmodified from
origin/main, and prints the table above. Fixtures for all three gaps plus the control were written
and executed; happy to attach them to a PR if someone picks this up.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions