From 44fe9a9e0e51f2df463632b41f6659ddb4bdea94 Mon Sep 17 00:00:00 2001 From: John Gruber Date: Wed, 19 Aug 2026 17:34:33 -0500 Subject: [PATCH 1/2] fix: version-derivation and release notes must read commit bodies, not just subjects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #177 review surfaced that compute_version_bump.sh scanned only commit subjects (%s). A conventional-commits BREAKING CHANGE is a *footer* — it lives in the body by definition — so the footer branch the code commented on was unreachable, and only `type!:` in a subject could ever produce a major bump. Concretely: 7ece9b04 (container-runner hardening, #2) declares in its body that the non-root gate now refuses named users — `USER nonroot` (the distroless convention) must become `USER 65532`. A real behavioural break, labelled as such by its author, that the tool couldn't see: it shipped as a patch, telling operators the opposite of the truth. The same %s-only blindness affected the release notes and CHANGELOG generation, so even had the version been right, the migration step would never have reached an operator. Fixes both: - compute_version_bump.sh reads each commit's subject AND body. Major is `type!:` in the subject or an uppercase `BREAKING CHANGE`/`BREAKING-CHANGE` marker anywhere; minor is `feat:` in the subject (type is declared in the subject, never the body). Case-sensitive on the marker so body prose like "not a breaking change" can't false-trigger now that bodies are read. - New scripts/extract-breaking-changes.sh emits a "⚠️ Breaking Changes" block (subject + the BREAKING CHANGE paragraph, so the migration text carries through). Wired into the RC notes, final release notes, and CHANGELOG steps; emits nothing when a range has no breaking change. Against v3.1.6..staging this now derives major → 4.0.0 (was silently 3.1.7) and surfaces the USER nonroot → USER 65532 migration. The bug outlived any single release: until the tool read bodies, every future footer-declared break would ship silently mis-versioned with its migration invisible. Claude-Session: https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4 --- .github/workflows/release.yml | 30 ++++++++++++++++++----- scripts/compute_version_bump.sh | 35 ++++++++++++++++----------- scripts/extract-breaking-changes.sh | 37 +++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 scripts/extract-breaking-changes.sh diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d874da5..0ded64e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -293,11 +293,17 @@ jobs: if [ -n "$LAST_FINAL" ]; then COMMIT_LOG=$(git log "${LAST_FINAL}..HEAD" --pretty=format:"- %s" | head -40) + BREAKING=$(bash scripts/extract-breaking-changes.sh "$LAST_FINAL" HEAD || true) else COMMIT_LOG=$(git log --pretty=format:"- %s" | head -40) + BREAKING="" fi - printf '%s\n\n%s' "$NOTES" "$COMMIT_LOG" > /tmp/rc_notes.md + if [ -n "$BREAKING" ]; then + printf '%s\n\n%s\n\n%s' "$NOTES" "$BREAKING" "$COMMIT_LOG" > /tmp/rc_notes.md + else + printf '%s\n\n%s' "$NOTES" "$COMMIT_LOG" > /tmp/rc_notes.md + fi echo "notes_file=/tmp/rc_notes.md" >> "$GITHUB_OUTPUT" - name: Create GitHub pre-release @@ -364,9 +370,11 @@ jobs: if [ -n "$LAST_FINAL" ]; then COMMITS=$(git log "${LAST_FINAL}..HEAD" --pretty=format:"- %s" \ | grep -v "^- release: " | head -50) + BREAKING=$(bash scripts/extract-breaking-changes.sh "$LAST_FINAL" HEAD || true) else COMMITS=$(git log --pretty=format:"- %s" \ | grep -v "^- release: " | head -50) + BREAKING="" fi # Build the entry in a temp file rather than interpolating COMMITS @@ -379,6 +387,10 @@ jobs: { echo "## v${NEW} (${DATE}) — ${BUMP} bump" echo "" + if [ -n "$BREAKING" ]; then + echo "$BREAKING" + echo "" + fi echo "$COMMITS" } > "$ENTRY_FILE" @@ -433,16 +445,22 @@ jobs: if [ -n "$LAST_FINAL" ]; then COMMITS=$(git log "${LAST_FINAL}..HEAD" --pretty=format:"- %s" \ | grep -v "^- release: " | head -50) + BREAKING=$(bash scripts/extract-breaking-changes.sh "$LAST_FINAL" HEAD || true) else COMMITS=$(git log --pretty=format:"- %s" \ | grep -v "^- release: " | head -50) + BREAKING="" fi - cat > /tmp/release_notes.md < /tmp/release_notes.md echo "notes_file=/tmp/release_notes.md" >> "$GITHUB_OUTPUT" diff --git a/scripts/compute_version_bump.sh b/scripts/compute_version_bump.sh index eca4369..27b4aff 100755 --- a/scripts/compute_version_bump.sh +++ b/scripts/compute_version_bump.sh @@ -88,15 +88,6 @@ else SINCE_TAG=$(last_final_tag) fi -if [[ -z "$SINCE_TAG" ]]; then - # No prior final tag — scan all commits. Whether this is actually safe is - # decided by the baseline resolution below (it requires an explicit - # --baseline in this case rather than silently proceeding). - COMMITS=$(git log --pretty=format:"%s" 2>/dev/null || true) -else - COMMITS=$(git log "${SINCE_TAG}..HEAD" --pretty=format:"%s" 2>/dev/null || true) -fi - if [[ -n "$BASELINE_OVERRIDE" ]]; then BASELINE="$BASELINE_OVERRIDE" elif [[ -n "$SINCE_TAG" ]]; then @@ -109,23 +100,39 @@ else fi # ── Determine bump type ─────────────────────────────────────────────────────── +# Conventional-commits: a breaking change is declared EITHER as `type!:` in the +# subject OR as a `BREAKING CHANGE` footer, which by definition lives in the +# body. The subject determines feat/fix. So we must read the body, not just the +# subject (%s) -- reading %s alone made the footer branch unreachable and shipped +# footer-declared breaking changes as patches (PR #177 review). BUMP_TYPE="patch" -while IFS= read -r subject; do - [[ -z "$subject" ]] && continue +if [[ -z "$SINCE_TAG" ]]; then + RANGE_HASHES=$(git log --pretty=format:"%H" 2>/dev/null || true) +else + RANGE_HASHES=$(git log "${SINCE_TAG}..HEAD" --pretty=format:"%H" 2>/dev/null || true) +fi + +while IFS= read -r sha; do + [[ -z "$sha" ]] && continue + subject=$(git log -1 --format="%s" "$sha" 2>/dev/null || true) + body=$(git log -1 --format="%b" "$sha" 2>/dev/null || true) - # BREAKING CHANGE in footer (multi-line) or ! in type - if echo "$subject" | grep -qiE '(\bBREAKING[[:space:]]+CHANGE\b|^[a-z]+(\([^)]*\))?!:)'; then + # Major: `type!:` in the subject, OR a BREAKING CHANGE / BREAKING-CHANGE marker + # anywhere in the message (footer or deliberate prose). + if echo "$subject" | grep -qE '^[a-z]+(\([^)]*\))?!:' \ + || printf '%s\n%s\n' "$subject" "$body" | grep -qE '\bBREAKING[[:space:] -]+CHANGE\b'; then BUMP_TYPE="major" break fi + # Minor: feat: in the subject (type is declared in the subject, never the body). if [[ "$BUMP_TYPE" != "major" ]]; then if echo "$subject" | grep -qE '^feat(\([^)]*\))?:'; then BUMP_TYPE="minor" fi fi -done <<< "$COMMITS" +done <<< "$RANGE_HASHES" # ── Compute target version ──────────────────────────────────────────────────── TARGET_VERSION=$(bump_version "$BASELINE" "$BUMP_TYPE") diff --git a/scripts/extract-breaking-changes.sh b/scripts/extract-breaking-changes.sh new file mode 100644 index 0000000..b0c71c6 --- /dev/null +++ b/scripts/extract-breaking-changes.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Emit a markdown "Breaking Changes" block for the commits in a range. +# +# Conventional-commits declares a breaking change as a `BREAKING CHANGE` footer, +# which lives in the commit BODY. The release notes / CHANGELOG generation reads +# only subjects (%s), so footer-declared breaks — and any migration steps they +# spell out — never reach operators (PR #177 review). This surfaces them. +# +# Usage: extract-breaking-changes.sh [] +# Prints a "### ⚠️ Breaking Changes" section, or nothing if there are none. +set -euo pipefail + +SINCE="${1:?usage: extract-breaking-changes.sh [until_ref]}" +UNTIL="${2:-HEAD}" + +block="" +while IFS= read -r sha; do + [[ -z "$sha" ]] && continue + body=$(git log -1 --format="%b" "$sha" 2>/dev/null || true) + # Uppercase footer/marker only (spec form), so body prose like "not a + # breaking change" does not false-trigger. + if printf '%s\n' "$body" | grep -qE '\bBREAKING[[:space:] -]+CHANGE\b'; then + subj=$(git log -1 --format="%s" "$sha" 2>/dev/null || true) + # The BREAKING CHANGE line and its paragraph (up to the next blank line), + # flattened to one line and stripped of markdown bold. + note=$(printf '%s\n' "$body" \ + | awk '/BREAKING[[:space:] -]+CHANGE/{p=1} p{print} p&&/^$/{exit}' \ + | tr '\n' ' ' | sed 's/\*\*//g; s/ */ /g; s/ *$//') + block="${block}- **${subj}** + ${note} +" + fi +done < <(git log "${SINCE}..${UNTIL}" --pretty=format:"%H" 2>/dev/null || true) + +if [[ -n "$block" ]]; then + printf '### ⚠️ Breaking Changes\n\n%s\n' "$block" +fi From 869cdc085755f3fe8b8132fe72ce2d88397357be Mon Sep 17 00:00:00 2001 From: John Gruber Date: Wed, 19 Aug 2026 17:37:15 -0500 Subject: [PATCH 2/2] test: cover the footer BREAKING CHANGE case; make the self-test self-contained MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds to the built-in SELF_TEST suite the case that was silently broken: - BREAKING CHANGE footer in the BODY of a fix-subject commit → major (non- vacuous: subject-only derivation computes patch here, so this fails against the pre-fix script). - lowercase "breaking change" in body prose → patch (guards the case-sensitive marker, so reading bodies can't false-positive on prose). The harness gains optional per-commit bodies (subject~~BODY~~body) since a footer only exists in a body, and sets a local git identity in its temp repos so the suite runs on a fresh runner with no global git config (it aborted otherwise). Also dropped a dead duplicate invocation and its stale COMMITS comment. SELF_TEST=1 bash scripts/compute_version_bump.sh → 6 passed. Claude-Session: https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4 --- scripts/compute_version_bump.sh | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/scripts/compute_version_bump.sh b/scripts/compute_version_bump.sh index 27b4aff..e888882 100755 --- a/scripts/compute_version_bump.sh +++ b/scripts/compute_version_bump.sh @@ -160,19 +160,30 @@ if [[ "${SELF_TEST:-0}" == "1" ]]; then local tmpdir tmpdir=$(mktemp -d) git init -q "$tmpdir" + # Self-contained identity so the self-test runs anywhere (fresh runners, + # no global git config). + git -C "$tmpdir" config user.email "selftest@bnk-forge.local" + git -C "$tmpdir" config user.name "bnk-forge self-test" git -C "$tmpdir" commit --allow-empty -m "initial" -q if [[ -n "$since" ]]; then git -C "$tmpdir" tag "$since" fi - # Add fake commits - while IFS='|' read -r msg; do - [[ -z "$msg" ]] && continue - git -C "$tmpdir" commit --allow-empty -m "$msg" -q + # Add fake commits. An entry may carry a body via "subject~~BODY~~body" + # so tests can exercise a footer-declared BREAKING CHANGE (bodies, not + # subjects, are where the spec puts it). Entries are comma-separated, so + # test strings must not contain commas. + while IFS= read -r entry; do + [[ -z "$entry" ]] && continue + if [[ "$entry" == *"~~BODY~~"* ]]; then + git -C "$tmpdir" commit --allow-empty \ + -m "${entry%%~~BODY~~*}" -m "${entry#*~~BODY~~}" -q + else + git -C "$tmpdir" commit --allow-empty -m "$entry" -q + fi done <<< "$(echo "$commits_str" | tr ',' '\n')" + # Run the version computer inside the temp repo so it scans the fake range. local result - result=$(bash "$(dirname "$0")/$(basename "$0")" --since-tag "${since:-}" --baseline "$baseline" 2>/dev/null || true) - # Override COMMITS via the temp repo by running in that dir result=$(cd "$tmpdir" && bash "$OLDPWD/$(dirname "$0")/$(basename "$0")" \ ${since:+--since-tag "$since"} --baseline "$baseline" 2>/dev/null || true) @@ -206,5 +217,15 @@ if [[ "${SELF_TEST:-0}" == "1" ]]; then # Test 4: no commits → patch bump run_test "no commits → patch" "patch" "1.2.4" "v1.2.3" "1.2.3" "" + # Test 5: BREAKING CHANGE footer in the BODY → major (the PR #177 bug: a + # fix-subject commit whose body declares the break must still bump major). + run_test "BREAKING CHANGE footer in body → major" "major" "2.0.0" "v1.2.3" "1.2.3" \ + "fix: harden non-root gate~~BODY~~BREAKING CHANGE: USER nonroot must become USER 65532" + + # Test 6: lowercase "breaking change" in body prose must NOT trigger major + # (case-sensitive marker, so reading bodies can't false-positive on prose). + run_test "lowercase breaking change in body → patch" "patch" "1.2.4" "v1.2.3" "1.2.3" \ + "fix: tidy up~~BODY~~this is explicitly not a breaking change" + echo "=== END SELF-TEST ===" fi