ci: gate changelogs on version order and duplicate versions

[kyle-sexton]

Why

#1758 renumbered a 3.1.1 entry that had reached main sitting below 4.0.0. This gate is the
reason that could happen at all, and it closes it.

The entry was authored against 3.1.0, and merged (#1686, 17:46:59Z) after 4.0.0 had already
landed (17:44:23Z). Its number was a regression the instant it merged. Nothing caught it, because
no gate reads a changelog as a sequence:

• --check asks whether a versioned plugin has a changelog at all.
• --check-bump asks whether this change set added an entry for its own new version.

Both reason about one version in isolation, so neither can see that a branch staged a number already
behind main, or that two branches staged the same one. A reviewer cannot see it either — the diff
hunk shows the new entry, never the resulting order.

That is not a one-off. The batch this came from had source-control 0.34.0 claimed by four branches
and work-items 0.26.0 by five; those were caught only because a human renumbered them by hand,
one merge at a time.

What this adds

scripts/check-changelog-parity.sh --check-order reads each changelog whole and fails on:

• a version sitting below a later one (naming the offending pair), and
• any version listed twice — the two-branches-staged-the-same-number case.

Wired into ci.yml as a non-PR-scoped step, because the defect is a property of the merged file
rather than of any one diff.

Scope note

It covers docs/conventions/*/CHANGELOG.md as well as plugins/*/CHANGELOG.md. That is deliberate
and load-bearing: convention changelogs carry no manifest version, so the other two modes never look
at them — and a convention changelog is exactly where this shipped.

Verification

Adversarial, not just green: the gate fails on main's pre-#1758 loop-lane changelog and passes
once the renumber is applied.

MISORDERED CHANGELOG: docs/conventions/loop-lane/CHANGELOG.md is not newest-first
  — 5.0.0 (below 3.1.1).

• check-changelog-parity.test.sh: 26 → 32 cases, 0 failures. Includes the exact shape that
  shipped (6.0.0 → 3.1.1 → 5.0.0 → 4.0.0, unbracketed convention headings), a duplicate-version
  case, and a 10.0.0 > 9.0.0 case so the comparison cannot regress to lexical.
• shellcheck -x on both scripts — clean, no suppressions added.
• --check-order across the repo: all 71 changelogs pass.

Both heading forms this repo uses are parsed: ## [1.2.3] (plugins) and ## 1.2.3 — date
(conventions). Comparison is sort -rV.

Related

No linked issue — this is the preventive half of #1758, which fixed the instance.

Refs #1758
Refs #1686

[claude]

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

---

Code Review

• Gather context and understand request
• Read changed files
• Review script logic and correctness
• Review test coverage
• Review CI integration
• Post findings

---

Overall this is a clean, well-motivated addition. The version-ordering logic is correct and the CI wiring is intentional. I found one formatting bug in the error message, a latent design coupling, and a minor test gap.

---

Bug — missing space in duplicate error message

scripts/check-changelog-parity.sh L108

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ')more than once. ..."

dupes is produced by command substitution ($(...)), which strips trailing newlines. So for a single duplicated version, dupes="2.0.0" (no trailing newline), tr '\n' ' ' is a no-op, and the output reads:

lists 2.0.0more than once.

The space before more is missing in every case because the trailing newline that tr would replace has been stripped. The test only asserts on *"DUPLICATE CHANGELOG VERSION"*, so it does not catch this.

Fix: add a literal space before more:

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ') more than once. ..."

Fix this →

---

Design coupling — manifest check gates --check-order unnecessarily

scripts/check-changelog-parity.sh L70–74

manifests=(plugins/*/.claude-plugin/plugin.json)
if [[ ! -e "${manifests[0]}" ]]; then
  echo "error: no plugin manifests found under plugins/*/.claude-plugin/" >&2
  exit 2
fi

This runs unconditionally before mode dispatch, so --check-order (which reads no manifests) would exit 2 with "no plugin manifests found" in a repo that has changelogs but no manifests. Not a problem for the current repo, but it's an odd invisible constraint on a mode that's explicitly designed to cover unversioned convention changelogs that exist independently of manifests.

Low priority — harmless for this repo — but worth noting if the check is ever extracted or reused.

---

Test gap — no coverage for changelogs with zero version headings

The ((${#versions[@]} > 1)) || continue guard skips changelogs with 0 or 1 entries (correct; nothing to order). A changelog with zero recognizable version headings (e.g., ## Unreleased only) would increment checked and silently pass. No test covers this. The behavior is likely intentional, but a quick comment in the test file naming it would make that clear to a future reader.

---

What looks good

• Version parsing regex correctly handles both repo heading forms (## [1.2.3] and ## 1.2.3 — date) via the two-pass grep. The ^ anchor prevents prose mentions from matching.
• sort -rV comparison is correct for semantic versioning and sidesteps the lexical 10 < 9 trap. The dedicated numeric test (10.0.0 > 9.0.0) confirms this.
• first_bad diagnostic is correct: if the sorted sequence differs from the original, the adjacent-pair loop is guaranteed to find the offending pair, so the message is always well-formed when emitted.
• CI wiring is right: --check-order has no if: github.event_name == 'pull_request' guard, which is intentional — the defect is a property of the merged file, not of any one diff.
• Scope covers docs/conventions/*/CHANGELOG.md, which is exactly where the bug that motivated this PR shipped. The test that reproduces the exact 6.0.0 → 3.1.1 → 5.0.0 → 4.0.0 shape is a good regression anchor.
• Duplicate detection with sort | uniq -d is correct (lexical sort is fine for equality; version sort isn't needed here).

[claude]

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

---

Security Review — PR #1762

• Read PR diff and changed files
• Analyze scripts/check-changelog-parity.sh for injection surfaces and logic flaws
• Analyze .github/workflows/ci.yml changes for dangerous patterns
• Analyze test script for any issues
• Report findings

---

No security issues found.

The review covered the four changed files through the lens of injection surfaces, logic/trust-boundary flaws, and dangerous GitHub Actions patterns. Findings per surface:

scripts/check-changelog-parity.sh — new --check-order block

Version extraction (the primary untrusted-data surface)

mapfile -t versions < <(grep -oE '^##[[:space:]]+\[?[0-9]+\.[0-9]+\.[0-9]+\]?' "$changelog" |
  grep -oE '[0-9]+\.[0-9]+\.[0-9]+')

The $changelog variable comes from bash glob expansion of the two literal patterns plugins/*/CHANGELOG.md and docs/conventions/*/CHANGELOG.md. Expanded paths are always prefixed with plugins/ or docs/conventions/, so they cannot start with - and cannot be mis-parsed as flags by grep. The double-pass regex (-oE only-matching mode, strict [0-9]+\.[0-9]+\.[0-9]+) means only digit-and-dot strings ever enter the versions array — no changelog content, including shell metacharacters, can survive the extraction.

Downstream uses of the extracted versions

sort -rV, uniq -d, printf '%s\n', and the head -1 comparison all receive values from ${versions[@]} or variables derived from it. Because those values can only be [0-9]+\.[0-9]+\.[0-9]+ strings, none of these calls carry injection risk.

Path use in error messages

$changelog and $dupes/$first_bad appear in echo ... >&2 lines. $changelog is a controlled filesystem path; the others are constructed from digit/dot version strings. No injection into the shell environment.

Glob non-expansion guard

If either glob produces no matches, bash leaves the unexpanded literal in the array (e.g., plugins/*/CHANGELOG.md). The [[ -f "$changelog" ]] || continue guard correctly skips the literal as a non-existent file, so the script exits cleanly with All 0 changelog(s) read newest-first.

.github/workflows/ci.yml — new step

- name: Verify every changelog reads newest-first with no duplicate versions
  run: scripts/check-changelog-parity.sh --check-order

The argument is a static string; no ${{ ... }} context interpolation is present. No attacker-controlled data flows into the run: block. The step does not widen permissions or reference any secrets.

scripts/check-changelog-parity.test.sh — new test block

Test cases use literal string arguments and mk_repo-generated temp directories. No dynamic or external input reaches shell execution paths.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95c2cab1bc

ℹ️ 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 2m 6s —— View job

---

Review (updated commit — a206bc9)

• Gather context
• Read changed files and diff
• Analyze new commit (a206bc9 fix(ci): compare changelog versions without sort -V)
• Post findings

---

The new commit is a clean, correct fix. Here's the full picture.

---

What the fix does

The first commit used sort -V for version ordering. CI correctly flagged it: sort -V is a GNU extension, and this repo's shell-portability-lint bans it. The fix replaces it with a version_sort_key() helper that zero-pads each semver field to five digits and uses a plain lexical > comparison.

version_sort_key() {
  local IFS='.'
  # shellcheck disable=SC2086  # deliberate word-split of a digits-only version on IFS
  set -- $1
  printf '%05d.%05d.%05d' "$((10#${1:-0}))" "$((10#${2:-0}))" "$((10#${3:-0}))"
}

Correctness: 10.0.0 → 00010.00000.00000 and 9.0.0 → 00009.00000.00000, so lexical > is numerically correct. The 10# prefix prevents octal interpretation of zero-padded results. The ${N:-0} defaults handle missing fields gracefully (though the upstream regex guarantees three fields). The SC2086 suppression is appropriate — the word-split on IFS='.' is the exact mechanism being used. No concerns here.

Comparison logic is now a single linear pass (O(n)) rather than a sort + comparison loop (O(n log n) + O(n)), and the error message is constructed inline, which is simpler and correct. The misordered counter still increments once per changelog thanks to break.

The 10.0.0 > 9.0.0 test case in the existing test suite covers the regression this would have introduced had the approach stayed lexical.

---

Still open from the previous review

Bug — missing space in duplicate error message (line 118)

This was flagged in the first review and is not addressed by this commit.

scripts/check-changelog-parity.sh L118

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ')more than once. ..."

Command substitution strips trailing newlines, so for a single duplicate version tr '\n' ' ' is a no-op and the output reads lists 2.0.0more than once.. Add a space before more:

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ') more than once. ..."

Fix this →

---

Codex finding — two-component convention versions not parsed

Per the inline Codex review, the regex [0-9]+\.[0-9]+\.[0-9]+ requires a patch component, so major.minor headings used in several convention changelogs (e.g., 1.2, 1.1, 1.0) are silently skipped. Those files pass --check-order while neither misordering nor duplicates would be caught in them. This is also not addressed in the current commit.

---

What looks good overall

• The sort -V removal is the right fix — no suppression, no platform workaround, the construct itself is resolved.
• The version_sort_key approach is idiomatic for portable semver comparison in shell and the boundary conditions (octal, missing fields) are handled explicitly.
• No changes to CI wiring, test scope, or the two changelog format patterns — the rest of the PR is unchanged.

[claude]

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

---

Security Review — PR #1762

• Gather context
• Read changed files
• Analyze check-changelog-parity.sh — injection surfaces, path handling, logic flaws
• Analyze ci.yml — workflow security
• Report findings

---

No security issues found.

The review covered the four changed files, focusing on logic flaws, authorization gaps, injection surfaces, unsafe data handling, and dangerous GitHub Actions patterns that static analysis cannot reach. Surfaces examined and cleared:

---

scripts/check-changelog-parity.sh — new --check-order block and version_sort_key

Version extraction (the primary untrusted-data surface)

mapfile -t versions < <(grep -oE '^##[[:space:]]+\[?[0-9]+\.[0-9]+\.[0-9]+\]?' "$changelog" |
  grep -oE '[0-9]+\.[0-9]+\.[0-9]+')

The double-pass filter is correct and tight. The second grep -oE is a strict [0-9]+\.[0-9]+\.[0-9]+ — only digit-and-dot strings can enter the versions array. All downstream consumers (version_sort_key, sort | uniq -d, printf, echo in error messages) receive values bounded to that character set. No changelog content, including any shell metacharacter, can survive the extraction.

version_sort_key word-split

version_sort_key() {
  local IFS='.'
  set -- $1
  printf '%05d.%05d.%05d' "$((10#${1:-0}))" "$((10#${2:-0}))" "$((10#${3:-0}))"
}

local IFS='.' is function-scoped — no global IFS pollution. $1 here is always a string matching [0-9]+\.[0-9]+\.[0-9]+ (traced above), so word-splitting on . yields three digit-only fields. The 10# prefix forces base-10 interpretation, preventing octal treatment of zero-padded fields like 09. No injection surface.

$changelog path use in grep call

Both glob patterns (plugins/*/CHANGELOG.md, docs/conventions/*/CHANGELOG.md) have literal path prefixes, so no expanded path can start with - and be misinterpreted as a flag. The [[ -f "$changelog" ]] || continue guard discards any unexpanded glob literal before it reaches grep. The variable is fully quoted in all grep invocations.

Error message construction

$changelog, $v, $prev, $dupes, and $first_bad all appear in echo … >&2 lines and are produced entirely from the [0-9]+\.[0-9]+\.[0-9]+ character class or controlled filesystem paths. No injection into the shell environment.

Fail-open gate check

The previous --check-bump code has a careful fail-closed guard against git diff returning a non-zero exit code. The new --check-order mode reads no git history — it only iterates a filesystem glob. If the glob matches zero files the loop body never executes and the script exits 0 (All 0 changelog(s) read newest-first). This is not exploitable: the manifest-presence check above the mode dispatch already ensures the repo has a plugin directory structure, and [[ -f "$changelog" ]] || continue means a glob that expands to nothing silently passes — not a failure mode that allows a real misordering to slip through.

---

.github/workflows/ci.yml — new step

- name: Verify every changelog reads newest-first with no duplicate versions
  run: scripts/check-changelog-parity.sh --check-order

The argument is a static string — no ${{ … }} context interpolation anywhere in this step. No attacker-controlled data enters the run: block. The step does not widen permissions (workflow-level permissions: contents: read is unchanged) and references no secrets. The intentional omission of an if: github.event_name == 'pull_request' guard is correct per the PR rationale.

---

scripts/check-changelog-parity.test.sh — new --check-order block

Test cases use literal string arguments and mktemp -d temp directories. All git operations target $repo (the temp dir), never the real repository. No dynamic or external input reaches shell execution paths. commit.gpgsign false in git_init is appropriate for an offline test harness.

---

[claude]

Claude finished @kyle-sexton's task in 1m 55s —— View job

---

Review (updated commit — c1885d4)

• Gather context and read changed files
• Analyze new commit (c1885d4 fix(ci): cover two-component convention changelog versions)
• Check whether previous open findings are addressed
• Post findings

---

The new commit is a clean, correct fix for the Codex P2 finding. Here's the full picture.

---

What the fix does

Several convention changelogs (permission-rule-hygiene, config-cascade, etc.) use major.minor headings like ## 1.2. The previous regex required a patch component ([0-9]+\.[0-9]+\.[0-9]+), so those files contributed zero versions to the ordering check — they were counted as "checked" while neither misordering nor duplicates could ever be caught in them.

The fix makes the patch component optional:

mapfile -t versions < <(grep -oE '^##[[:space:]]+\[?[0-9]+\.[0-9]+(\.[0-9]+)?\]?([[:space:]]|$)' "$changelog" |
  grep -oE '[0-9]+\.[0-9]+(\.[0-9]+)?')

Regex correctness: The trailing ([[:space:]]|$) anchor is the important load-bearing piece — it prevents ## 1.2.3.4 from matching as 1.2.3 by requiring whitespace or end-of-line after the version. Correct.

Comparison compatibility: version_sort_key already handled the two-component case via ${3:-0} (missing patch defaults to 0). No change was needed and none was made.

Mixed-width correctness: 1.10 → 00001.00010.00000 outranks 1.9.1 → 00001.00009.00001, which outranks 1.9 → 00001.00009.00000. The new mixed-width test at line 475 confirms this (1.10 > 1.9.1 > 1.9 passes).

New test cases are well-chosen: descending two-component pass (line 467), misordered two-component catch (line 470), and the mixed-width comparison (line 475). Together they eliminate the silent-pass scenario the Codex finding described.

---

Still open — missing space in duplicate error message (line 121)

This was flagged in the first two review rounds and is not addressed by this commit.

scripts/check-changelog-parity.sh L121

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ')more than once. ..."

Command substitution strips trailing newlines. For a single duplicate version dupes="2.0.0" (no trailing newline), tr '\n' ' ' is a no-op, and the output reads:

lists 2.0.0more than once.

Fix — add a literal space before more:

echo "DUPLICATE CHANGELOG VERSION: $changelog lists $(printf '%s' "$dupes" | tr '\n' ' ') more than once. ..."

The test at line 444 only asserts *"DUPLICATE CHANGELOG VERSION"*, so it does not catch this.

Fix this →

---

What looks good overall

• The Codex P2 finding is fully resolved: two-component versions are now parsed, ordered, and tested.
• The ([[:space:]]|$) tail anchor in the expanded regex is correct and prevents ## 1.2.3.4 from silently matching as 1.2.3.
• version_sort_key needed no change — the ${3:-0} default already handled missing patch fields.
• All three prior open issues have now been addressed except the duplicate-message space.

[claude]

Claude finished @kyle-sexton's task in 1m 36s —— View job

---

Security Review — PR #1762 (updated commit c1885d4)

• Gather context
• Read PR diff and changed files
• Analyze check-changelog-parity.sh for injection surfaces and logic flaws
• Analyze .github/workflows/ci.yml changes for dangerous patterns
• Report findings

---

No security issues found.

This review covers the third commit (c1885d4 fix(ci): cover two-component convention changelog versions), which extends the --check-order regex to also accept major.minor headings. The prior two commits were reviewed at 95c2cab and a206bc9; this pass focuses on what changed in this commit.

---

Change scope

The commit has two effects:

1. Regex updated — the version-extraction pattern becomes:

   mapfile -t versions < <(grep -oE '^##[[:space:]]+\[?[0-9]+\.[0-9]+(\.[0-9]+)?\]?([[:space:]]|$)' "$changelog" |
     grep -oE '[0-9]+\.[0-9]+(\.[0-9]+)?')

2. version_sort_key now receives two-component inputs — a major.minor version like 1.2 must round-trip through the zero-padding helper.

---

Security analysis

Version extraction — the primary untrusted-data surface

The updated regex makes the optional patch field explicit ((\.[0-9]+)?) and adds a trailing anchor ([[:space:]]|$). The anchor is a tightening: a heading like ## 1.2abc would fail the trailing guard and produce no match. Tracing ## 1.2.3.4 through the pattern: after matching 1.2.3 the pattern requires ([[:space:]]|$) but encounters .4, so no match. Only [0-9]+\.[0-9]+(\.[0-9]+)? strings ever survive the two-pass extraction — the character class is unchanged from the previous commit.

version_sort_key with two-component inputs

With IFS='.' and set -- $1 on 1.2, positional parameters become $1=1 $2=2 $3= (unset). The ${3:-0} default substitutes 0, yielding 00001.00002.00000. The test case 1.10 > 1.9.1 (00001.00010.00000 > 00001.00009.00001 lexically) confirms the mixed-width comparison is numerically correct. No injection surface — input is bounded to [0-9]+ fields by the extraction stage.

Glob patterns and path handling

Unchanged from the prior commit. No new filesystem paths or variable interpolation into command arguments.

CI step

Unchanged — still a static scripts/check-changelog-parity.sh --check-order with no ${{ ... }} interpolation.

Test script additions

New test cases use write_changelog with literal strings and the existing $repo temp directory. No dynamic or external input reaches shell execution paths.

---