Skip to content

Commit 122a6c2

Browse files
metcalfcclaude
metcalfc
and
claude
authored
Base the changelog on the previous release of the same line, and assert it (#479)
* fix: base the changelog on the previous release of the same line, and assert it Two halves of the same problem: the release notes were computed from the wrong base, and nothing in CI would have noticed. base-ref was left empty, so the action asked the API for the latest release -- which is the highest release across every line. Cutting v4.9.1 therefore diffed it against v5.0.0 and produced a symmetric difference listing commits from both lines; two of its seven entries were real. Ask git for the previous release tag reachable from this commit instead. That answers v4.9.0 for a v4 patch and v4.8.0 for v5.0.0, because the v4 line is not reachable from main. --match skips the moving major tags, which sit on the same commits as the release tags. The checkout needs fetch-depth: 0 for git describe to see any of this. An empty result still falls back to the API, which is right for a repository with a single line. The end-to-end job generated four changelogs and only printed them, which is how it stayed green while emitting a single line of literal %0A. It now compares the frozen v0.0.1..v0.0.2 range exactly, checks that reverse: true reverses and reverse: false matches the default, rejects percent-encoded newlines by name, and shape-checks the release-based changelog. Outputs reach the script through the environment rather than being pasted into it, since a commit subject is untrusted. Verified each assertion fails against the bug it exists for, including the original %0A regression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015EmepbnR2nSdk8q8DBsrsD * fix: build the modified changelog from the environment The assertion added alongside caught two artifacts of interpolating the changelog into a heredoc. The value's trailing newline became an empty final line that tac moved to the front, and the ${{ }} token sits on an indented YAML line, so the value's first line picked up ten leading spaces -- enough for Markdown to render it as a code block. That one survived only because the line it landed on happens to say Bumping and is grepped away. Read the value from the environment and drop blank lines. README carried the same recipe. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0bee5ad commit 122a6c2

4 files changed

Lines changed: 124 additions & 10 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,32 @@ jobs:
5252
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # was: actions/checkout@v6
5353
with:
5454
ref: ${{ github.sha }}
55+
# git describe below needs the history and the tags, neither of which
56+
# the default shallow, tagless checkout provides.
57+
fetch-depth: 0
58+
- name: Find the previous release on this line
59+
id: previous
60+
run: |
61+
set -euo pipefail
62+
# Left to itself the action asks the API for the latest release, and
63+
# that is the highest release across every line -- so a v4 patch
64+
# diffed against v5 and produced a symmetric difference listing
65+
# commits from both lines. Ask git for the previous release tag
66+
# reachable from this commit instead, which is right on either line.
67+
# --match skips the moving major tags (v4, v5), which point at the
68+
# same commits as the release tags and would otherwise be ambiguous.
69+
tag=$(git describe --tags --abbrev=0 \
70+
--match 'v[0-9]*.[0-9]*.[0-9]*' "$GITHUB_SHA^" 2>/dev/null || true)
71+
echo "tag=$tag" >> "$GITHUB_OUTPUT"
72+
echo "previous release: ${tag:-<none: falling back to the latest release>}"
5573
- name: Generate changelog
5674
id: changelog
5775
uses: ./
5876
with:
5977
myToken: ${{ secrets.GITHUB_TOKEN }}
78+
# Empty falls back to the API's latest release, which is the old
79+
# behaviour and the right answer for a repository with one line.
80+
base-ref: ${{ steps.previous.outputs.tag }}
6081
- name: Verify attestation subject is unchanged
6182
run: git diff --exit-code "$GITHUB_SHA" -- dist/index.js dist/changelog.sh
6283
- name: Verify release tag still targets this commit

‎.github/workflows/test.yml‎

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,16 @@ jobs:
6161
EOF
6262
- name: Modify the changelog
6363
id: modified
64+
# The changelog arrives by environment rather than being pasted into
65+
# the script. Interpolating it into a heredoc prepends this block's
66+
# YAML indentation to the value's first line -- ten spaces, which
67+
# Markdown renders as a code block -- and appends a blank line from the
68+
# value's trailing newline, which tac then moves to the front.
69+
env:
70+
CHANGELOG: ${{ steps.changelog.outputs.changelog }}
6471
run: |
65-
set -o noglob
66-
log=$(cat << "EOF" | grep -v Bumping | tac
67-
${{ steps.changelog.outputs.changelog }}
68-
EOF
69-
)
72+
set -euo pipefail
73+
log=$(printf '%s\n' "$CHANGELOG" | grep -v Bumping | grep -v '^[[:space:]]*$' | tac)
7074
# A random delimiter, per GitHub's own guidance: the value is built
7175
# from commit subjects, so a fixed marker is something a commit could
7276
# contain in order to write extra keys into GITHUB_OUTPUT.
@@ -91,3 +95,67 @@ jobs:
9195
cat << "EOF"
9296
${{ steps.release.outputs.changelog }}
9397
EOF
98+
- name: Assert the generated changelogs
99+
# Every step above only printed its output, so this job passed while
100+
# emitting a single line of literal %0A for as long as that bug lived.
101+
# The outputs arrive through the environment rather than being pasted
102+
# into the script: a commit subject is untrusted, and this is the same
103+
# boundary the release workflow keeps for tag names.
104+
env:
105+
CHANGELOG: ${{ steps.changelog.outputs.changelog }}
106+
REVERSED: ${{ steps.changelog-rev.outputs.changelog }}
107+
NOT_REVERSED: ${{ steps.changelog-notrev.outputs.changelog }}
108+
MODIFIED: ${{ steps.modified.outputs.modified }}
109+
FROM_RELEASE: ${{ steps.release.outputs.changelog }}
110+
run: |
111+
set -euo pipefail
112+
113+
check() {
114+
if [ "$2" = "$3" ]; then
115+
echo "ok: $1"
116+
else
117+
echo "::error::$1"
118+
printf 'got:\n%s\n\nwant:\n%s\n' "$2" "$3"
119+
exit 1
120+
fi
121+
}
122+
123+
# v0.0.1..v0.0.2 is a frozen three-commit range, so the entire
124+
# output is deterministic and can be compared exactly rather than
125+
# pattern-matched.
126+
# The backticks are literal Markdown from the code span, not command
127+
# substitution, so single quotes are exactly right here.
128+
# shellcheck disable=SC2016
129+
one='- [554162a](http://github.com/metcalfc/changelog-generator/commit/554162af5681a3e485056d52716e0a8a6e868510) - ` Bumping to 0.0.2 `'
130+
# shellcheck disable=SC2016
131+
two='- [8ea6bc3](http://github.com/metcalfc/changelog-generator/commit/8ea6bc3e7e1e973498683897fe624797f1f54f54) - ` Javascript not my strong suit. `'
132+
# shellcheck disable=SC2016
133+
three='- [c74bcf9](http://github.com/metcalfc/changelog-generator/commit/c74bcf9663574361b90225d226b952a769266daf) - ` Fix README example spacing. `'
134+
135+
# $() strips trailing newlines from both sides, so the comparison
136+
# does not hinge on whether the output ends with one.
137+
check 'default order' \
138+
"$(printf '%s' "$CHANGELOG")" "$(printf '%s\n%s\n%s' "$one" "$two" "$three")"
139+
check 'reverse: false matches the default' \
140+
"$(printf '%s' "$NOT_REVERSED")" "$(printf '%s' "$CHANGELOG")"
141+
check 'reverse: true reverses' \
142+
"$(printf '%s' "$REVERSED")" "$(printf '%s\n%s\n%s' "$three" "$two" "$one")"
143+
144+
# grep -v Bumping drops the first entry and tac flips the rest. This
145+
# is the step that silently emitted %0A instead of newlines.
146+
check 'the modified changelog is real multiline text' \
147+
"$(printf '%s' "$MODIFIED")" "$(printf '%s\n%s' "$three" "$two")"
148+
149+
case "$MODIFIED" in
150+
*%0A* | *%25* | *%0D*)
151+
echo "::error::percent-encoded newlines are back in the output"
152+
exit 1
153+
;;
154+
esac
155+
156+
# This one has no fixed range -- it runs against the latest release --
157+
# so assert the shape instead of the content.
158+
[ -n "$FROM_RELEASE" ] || { echo "::error::empty release changelog"; exit 1; }
159+
bad=$(printf '%s' "$FROM_RELEASE" | grep -cvE '^- \[[0-9a-f]+\]\(http://github\.com/[^)]+\) - .+$' || true)
160+
[ "$bad" = "0" ] || { echo "::error::$bad malformed line(s)"; printf '%s\n' "$FROM_RELEASE"; exit 1; }
161+
echo "ok: release-based changelog is well formed"

‎README.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,11 @@ In order to keep this action as simple as possible we aren't planning to add mor
123123
```yaml
124124
- name: Modify the changelog
125125
id: modified
126+
env:
127+
CHANGELOG: ${{ steps.changelog.outputs.changelog }}
126128
run: |
127-
set -o noglob
128-
log=$(cat << "EOF" | grep -v Bumping | tac
129-
${{ steps.changelog.outputs.changelog }}
130-
EOF
131-
)
129+
set -euo pipefail
130+
log=$(printf '%s\n' "$CHANGELOG" | grep -v Bumping | grep -v '^[[:space:]]*$' | tac)
132131
delimiter=$(openssl rand -hex 16)
133132
{
134133
echo "log<<$delimiter"
@@ -145,6 +144,8 @@ In order to keep this action as simple as possible we aren't planning to add mor
145144

146145
That heredoc is how you return a multiline value. A plain `echo "log=$log" >> $GITHUB_OUTPUT` keeps only the first line.
147146

147+
The changelog is read from the environment rather than interpolated into the script. `${{ }}` inside a `run:` block is textual substitution, so a value pasted into a heredoc picks up that block's YAML indentation on its first line -- which Markdown then renders as a code block -- and a commit subject is untrusted input besides.
148+
148149
This example used to percent-encode the newlines as `%0A` instead, which the long-gone `::set-output` command decoded. `$GITHUB_OUTPUT` does not, so that version produced a single line with literal `%0A` in it. Use a random delimiter rather than a fixed one: the value is built from commit subjects, and a fixed marker is something a commit subject could contain in order to write additional keys into `$GITHUB_OUTPUT`.
149150

150151
## Example use case

‎test/release-integrity.test.mjs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,27 @@ test('release attestation is gated on exact-revision dist verification', () => {
4747
assert.match(releaseJob, /commit: \$\{\{\s*github\.sha\s*\}\}/)
4848
assert.match(releaseJob, /immutableCreate: true/)
4949
})
50+
51+
test('the changelog is based on the previous release of the same line', () => {
52+
const releaseJob = workflow.slice(workflow.indexOf(' release:'))
53+
54+
// The API's latest release is the highest across every line, so a v4 patch
55+
// diffed against v5 and listed commits from both. git describe answers with
56+
// the previous release reachable from this commit, which is right on either.
57+
assert.match(releaseJob, /fetch-depth: 0/)
58+
assert.match(releaseJob, /git describe --tags --abbrev=0/)
59+
assert.match(
60+
releaseJob,
61+
/base-ref: \$\{\{ steps\.previous\.outputs\.tag \}\}/
62+
)
63+
64+
// Without --match, the moving major tags (v4, v5) sit on the same commits as
65+
// the release tags and describe may answer with one of those instead.
66+
assert.match(releaseJob, /--match 'v\[0-9\]\*\.\[0-9\]\*\.\[0-9\]\*'/)
67+
68+
// The lookup must precede the step that consumes it.
69+
assert.ok(
70+
releaseJob.indexOf('id: previous') <
71+
releaseJob.indexOf('base-ref: ${{ steps.previous.outputs.tag }}')
72+
)
73+
})

0 commit comments

Comments
 (0)