Skip to content

Commit c71c827

Browse files
authored
fix(release): never publish an empty changelog (commit-based fallback) (#1766)
* fix(release): fall back to a commit-based changelog when generate-notes finds no PRs releases/generate-notes aggregates merged pull requests over the compared tag range. Work landing as direct commits on dev (or through PRs based on dev rather than the release branch) leaves that range with nothing to aggregate, so the body collapsed to the npm line plus a compare link: v2.17.0..v2.18.2 had 0 of 36 commits PR-associated and v2.18.2..v2.19.0 had 2 of 21, and both releases shipped a 169-char stub. The workflow now detects an empty PR delta and renders the commit log instead, categorized by conventional-commit prefix, excluding merge and release-bump commits. The renderer keeps only PR-numbered entries, so the fallback travels on its own --commit-fallback channel and is emitted only when the PR pipeline produced no sections at all. * fix(release): harden the commit fallback against untrusted commit metadata Audit findings on the first commit: (1) a preview whose notes came from the fallback would carry into a stable release as meaningful, but the PR renderer discards non-PR bullets, collapsing the stable body back to the stub; (2) %an is a free-form git display name, so a contributor named e.g. Abhishek Sharma rendered as a live @abhishek mention, and subjects could inject markdown or a forged unit separator; (3) real merges using the merge: conventional prefix were not treated as plumbing. extractCommitBulletSections carries PR-free bullets through the renderer, sanitizeCommitText neutralizes mentions/markdown/separators, authors render as plain text in a (sha, Name) trailer, non-hex shas are dropped, parseCommitLog splits the author from the right, and merge: joins the plumbing filter. Nine regression tests cover each. * fix(release): NUL-frame the commit log and decouple the fallback from carried notes Second audit round: (1) U+001F is legal in git author names too, so right-splitting still let author metadata shift the subject; the log is now NUL-delimited (git log -z), which git forbids inside commit content, and records parse as exactly three fields. (2) The workflow gated fallback generation on carried notes as well as the delta, so a stable release carrying commit-based preview notes silently dropped every post-preview direct commit; the guard now depends on this range delta only and the renderer decides whether to emit. (3) The sanitizer escaped by deletion, mangling technical text like Map<K, V> | CLI; metacharacters are now backslash-escaped instead. * fix(release): merge fallback sections by category and de-vacuum two tests Third audit round: concatenating carried and current fallback bodies emitted a duplicate heading when both touched the same category; mergeCommitBulletSections now merges by category with bullet de-duplication. Two converted tests still carried literal \\u001f fixtures, so parseCommitLog returned nothing and they asserted on an empty string; both now use real NUL fields and assert the entry renders. * docs(release): put the commit-log JSDoc back on parseCommitLog
1 parent b1b05b8 commit c71c827

3 files changed

Lines changed: 518 additions & 2 deletions

File tree

.github/workflows/release.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,8 +374,10 @@ jobs:
374374
notes_file="$(mktemp)"
375375
carried_file="$(mktemp)"
376376
delta_file="$(mktemp)"
377+
commit_fallback_file="$(mktemp)"
377378
: > "$carried_file"
378379
: > "$delta_file"
380+
: > "$commit_fallback_file"
379381
380382
# Stable releases after matching previews: aggregate every matching preview
381383
# changelog (oldest→newest; each preview body is incremental), then only
@@ -449,6 +451,31 @@ jobs:
449451
pr_notes="$(gh api "${generate_notes_api[@]}" --jq '.body')"
450452
# Drop generate-notes' trailing compare link; we re-append it after the commit list.
451453
printf '%s\n' "$pr_notes" | sed '/^\*\*Full Changelog\*\*:/d' > "$delta_file"
454+
455+
# generate-notes counts MERGED PULL REQUESTS in the tag range. Work that lands
456+
# as direct commits on the integration branch (or via PRs based on `dev` rather
457+
# than this release branch) leaves that range with nothing to aggregate, and the
458+
# body collapses to the npm line plus a compare link — v2.17.0..v2.18.2 shipped
459+
# exactly that, 0 of 36 commits PR-associated. Fall back to the commit log so a
460+
# release can never publish an empty changelog.
461+
# The renderer only keeps entries carrying a PR number, so the fallback
462+
# travels in its own channel (--commit-fallback). The decision depends on
463+
# THIS range's PR delta only: carried preview notes cover the pre-preview
464+
# span, so gating on them too would silently drop every post-preview
465+
# direct commit. The renderer decides whether to emit the channel.
466+
if ! bun scripts/release-notes.ts has-meaningful "$delta_file"; then
467+
commit_log_file="$(mktemp)"
468+
# NUL-delimited: Git forbids NUL in commit content, so neither a crafted
469+
# subject nor an author name can forge a field boundary.
470+
git log -z --format='%H%x00%s%x00%an' "${notes_range_start}..${GITHUB_SHA}" > "$commit_log_file"
471+
bun scripts/release-notes.ts commit-fallback "$commit_log_file" > "$commit_fallback_file"
472+
if bun scripts/release-notes.ts has-meaningful "$commit_fallback_file"; then
473+
echo "::notice::generate-notes returned no PR categories for ${notes_range_start}..${release_tag}; using the commit-based changelog fallback"
474+
else
475+
: > "$commit_fallback_file"
476+
echo "::notice::No PR categories and no eligible commits in ${notes_range_start}..${release_tag}; release notes stay minimal"
477+
fi
478+
fi
452479
else
453480
# First release on this channel: never call generate-notes without previous_tag_name.
454481
# GitHub would baseline the newest repo tag, which may belong to the other channel.
@@ -476,6 +503,7 @@ jobs:
476503
--npm-metadata "$npm_metadata"
477504
--carried "$carried_file"
478505
--delta "$delta_file"
506+
--commit-fallback "$commit_fallback_file"
479507
--out "$notes_file"
480508
--compare-to "$release_tag"
481509
--repository "$GITHUB_REPOSITORY"

scripts/release-notes.ts

Lines changed: 248 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* bun scripts/release-notes.ts matching-preview-tags <version>
1111
* bun scripts/release-notes.ts previous-release-tag <version>
1212
* bun scripts/release-notes.ts has-meaningful [body-file]
13+
* bun scripts/release-notes.ts commit-fallback [commit-log-file]
1314
* bun scripts/release-notes.ts credit-takeovers --repo <owner/name> --in <file> --out <file>
1415
* bun scripts/release-notes.ts render --npm-metadata ... --out ... [--carried ...] [--delta ...] [--compare-from ...] [--compare-to ...] [--repository ...]
1516
* bun scripts/release-notes.ts polish --in <file> --out <file> [--model ...] [--base-url ...]
@@ -177,6 +178,223 @@ export function hasMeaningfulCarriedNotes(stripped: string): boolean {
177178
return !isEmptyGeneratedNotes(stripped);
178179
}
179180

181+
/**
182+
* A single commit considered for the commit-based changelog fallback.
183+
* `sha` is the full or short hash; `subject` is the commit subject line.
184+
*/
185+
export type ReleaseNoteCommit = {
186+
sha: string;
187+
subject: string;
188+
author: string;
189+
};
190+
191+
/** Category order shared by the PR renderer and the commit fallback. */
192+
const RENDER_CATEGORY_ORDER = ["New Features", "Bug Fixes", "Documentation", "Chores", "Other Changes"];
193+
194+
/** Conventional-commit type -> release.yml category title. */
195+
const COMMIT_TYPE_CATEGORY: Record<string, string> = {
196+
feat: "New Features",
197+
fix: "Bug Fixes",
198+
perf: "Bug Fixes",
199+
docs: "Documentation",
200+
chore: "Chores",
201+
build: "Chores",
202+
ci: "Chores",
203+
refactor: "Chores",
204+
style: "Chores",
205+
test: "Chores",
206+
};
207+
208+
/**
209+
* Commits that are release plumbing rather than shipped work. A merge commit's
210+
* content is already represented by the commits it brings in, and a `release:`
211+
* bump is the release itself.
212+
*/
213+
export function isReleasePlumbingCommit(subject: string): boolean {
214+
const text = subject.trim();
215+
if (/^Merge\s/i.test(text)) return true;
216+
// Real two-parent merges in this repo also use a `merge:` conventional prefix.
217+
if (/^merge(?:\([^)]*\))?!?:\s/i.test(text)) return true;
218+
if (/^release(?:\([^)]*\))?!?:\s/i.test(text)) return true;
219+
return false;
220+
}
221+
222+
/**
223+
* Neutralize Markdown and mention syntax from untrusted commit text before it
224+
* lands in a release body. Commit subjects and author names are attacker- or
225+
* accident-controlled: a bare `@name` renders as a real GitHub mention (and
226+
* notifies that account), and backticks/brackets can restructure the notes.
227+
*/
228+
export function sanitizeCommitText(text: string): string {
229+
return text
230+
.replace(/\r?\n/g, " ")
231+
// Strip the ASCII unit separator so a subject can never forge a log field.
232+
.replace(/[\u0000\u001f]/g, " ")
233+
// Escape rather than delete: `Map<K, V> | CLI` must stay readable.
234+
.replace(/([`<>|[\]\\])/g, "\\$1")
235+
// `@name` -> `@\u200bname`: reads identically, never notifies.
236+
.replace(/@(?=[A-Za-z0-9_-])/g, "@\u200b")
237+
.replace(/\s+/g, " ")
238+
.trim();
239+
}
240+
241+
/**
242+
* Render commits as a generate-notes-shaped body so the existing category
243+
* parser/renderer can consume them unchanged.
244+
*
245+
* Why this exists: `releases/generate-notes` aggregates MERGED PULL REQUESTS
246+
* against the compared tag range. When work lands as direct commits on the
247+
* integration branch (or through PRs whose base is `dev` rather than the
248+
* release branch), that range contains no PRs the API will count and the body
249+
* collapses to the npm line plus a compare link — v2.17.0..v2.18.2 had 0 of 36
250+
* commits associated with a main-merged PR, and both releases shipped an empty
251+
* changelog. The fallback keeps the release body honest regardless of how the
252+
* work reached the branch.
253+
*
254+
* Commits carry no PR number, so the synthetic entries use `#0` — a sentinel
255+
* the renderer never prints as a link because these are emitted as plain
256+
* bullets under their category heading.
257+
*/
258+
export function renderCommitFallbackNotes(commits: ReleaseNoteCommit[]): string {
259+
const buckets = new Map<string, string[]>();
260+
for (const commit of commits) {
261+
const subject = commit.subject.trim();
262+
if (!subject) continue;
263+
if (isReleasePlumbingCommit(subject)) continue;
264+
const match = /^([a-zA-Z]+)(?:\(([^)]*)\))?!?:\s*(.+)$/.exec(subject);
265+
const type = match?.[1]?.toLowerCase();
266+
const scope = sanitizeCommitText(match?.[2] ?? "");
267+
const summary = sanitizeCommitText(match?.[3] ?? subject);
268+
if (!summary) continue;
269+
const category = (type && COMMIT_TYPE_CATEGORY[type]) ?? "Other Changes";
270+
// Hex-only short hash: a crafted `sha` field can never inject markup.
271+
const shortSha = /^[0-9a-f]{7,40}$/i.test(commit.sha.trim())
272+
? commit.sha.trim().slice(0, 9)
273+
: "";
274+
const scopePrefix = scope ? `${scope}: ` : "";
275+
// `%an` is a free-form Git display name, not a GitHub login, so it is
276+
// rendered as plain text rather than an @mention that would notify a
277+
// same-named (or non-existent) account.
278+
const author = sanitizeCommitText(commit.author).replace(/^@\u200b/, "");
279+
const trailer = [shortSha, author].filter(Boolean).join(", ");
280+
const line = trailer ? `- ${scopePrefix}${summary} (${trailer})` : `- ${scopePrefix}${summary}`;
281+
const existing = buckets.get(category);
282+
if (existing) existing.push(line);
283+
else buckets.set(category, [line]);
284+
}
285+
if (buckets.size === 0) return "";
286+
const parts: string[] = [];
287+
for (const title of RENDER_CATEGORY_ORDER) {
288+
const lines = buckets.get(title);
289+
if (!lines || lines.length === 0) continue;
290+
parts.push([`## ${title}`, "", ...lines].join("\n"));
291+
}
292+
return parts.join("\n\n").replace(/\n+$/, "") + "\n";
293+
}
294+
295+
/**
296+
* Extract commit-style category sections (bullets with no `(#N)` reference)
297+
* from an already-rendered body.
298+
*
299+
* A preview release whose notes came from the commit fallback carries bullets
300+
* like `- gui: fix a thing (abc1234, Name)`. Those are meaningful prose, so the
301+
* workflow keeps them as carried notes and skips regenerating a fallback — but
302+
* the PR renderer only retains entries carrying a PR number, so without this
303+
* the stable release would silently collapse back to the npm-line stub.
304+
*/
305+
export function extractCommitBulletSections(body: string): string {
306+
const out: string[] = [];
307+
let current: { title: string; lines: string[] } | null = null;
308+
const flush = (): void => {
309+
if (current && current.lines.length > 0) {
310+
out.push([`## ${current.title}`, "", ...current.lines].join("\n"));
311+
}
312+
current = null;
313+
};
314+
for (const rawLine of body.replace(/\r\n/g, "\n").split("\n")) {
315+
const line = rawLine.trim();
316+
if (!line || line.startsWith("<!--")) continue;
317+
if (line.startsWith("## ") || line.startsWith("### ")) {
318+
flush();
319+
const title = line.replace(/^#{2,3}\s+/, "").trim();
320+
if (!SCAFFOLD_HEADINGS.has(title)) current = { title, lines: [] };
321+
continue;
322+
}
323+
if (!current) continue;
324+
if (!line.startsWith("- ")) continue;
325+
// Anything carrying a PR reference belongs to the PR pipeline, not here.
326+
if (/\(#\d+(?:\s*,\s*#\d+)*\)\s*$/.test(line)) continue;
327+
if (/^-\s+#\d+\s/.test(line)) continue;
328+
current.lines.push(line);
329+
}
330+
flush();
331+
return out.join("\n\n").replace(/\n+$/, "") + (out.length > 0 ? "\n" : "");
332+
}
333+
334+
/**
335+
* Merge several already-rendered commit-bullet bodies into one set of category
336+
* sections, preserving order within a category and de-duplicating identical
337+
* bullets. Concatenating the bodies directly would repeat a shared heading.
338+
*/
339+
export function mergeCommitBulletSections(bodies: string[]): string {
340+
const buckets = new Map<string, string[]>();
341+
const seen = new Set<string>();
342+
for (const body of bodies) {
343+
let current: string | null = null;
344+
for (const rawLine of (body ?? "").replace(/\r\n/g, "\n").split("\n")) {
345+
const line = rawLine.trim();
346+
if (!line) continue;
347+
if (line.startsWith("## ") || line.startsWith("### ")) {
348+
current = line.replace(/^#{2,3}\s+/, "").trim();
349+
if (!buckets.has(current)) buckets.set(current, []);
350+
continue;
351+
}
352+
if (!current || !line.startsWith("- ")) continue;
353+
const key = `${current}\u0000${line}`;
354+
if (seen.has(key)) continue;
355+
seen.add(key);
356+
buckets.get(current)!.push(line);
357+
}
358+
}
359+
const titles = [...buckets.keys()].sort((x, y) => {
360+
const ix = RENDER_CATEGORY_ORDER.indexOf(x);
361+
const iy = RENDER_CATEGORY_ORDER.indexOf(y);
362+
const rx = ix === -1 ? RENDER_CATEGORY_ORDER.length : ix;
363+
const ry = iy === -1 ? RENDER_CATEGORY_ORDER.length : iy;
364+
return rx - ry;
365+
});
366+
const merged: string[] = [];
367+
for (const title of titles) {
368+
const lines = buckets.get(title)!;
369+
if (lines.length === 0) continue;
370+
merged.push([`## ${title}`, "", ...lines].join("\n"));
371+
}
372+
return merged.join("\n\n").trim();
373+
}
374+
375+
/**
376+
* Parse `git log -z --format=%H%x00%s%x00%an` output into commits.
377+
*
378+
* Records and fields are NUL-separated. Git forbids NUL in commit content, so
379+
* — unlike the unit separator, which Git accepts in both subjects and author
380+
* names — no field value can forge a boundary. Every record is read as exactly
381+
* three fields.
382+
*/
383+
export function parseCommitLog(raw: string): ReleaseNoteCommit[] {
384+
const commits: ReleaseNoteCommit[] = [];
385+
const fields = raw.split("\u0000");
386+
// Trailing separator from `git log -z` leaves an empty final element.
387+
if (fields.length > 0 && fields[fields.length - 1]!.trim() === "") fields.pop();
388+
for (let i = 0; i + 2 < fields.length + 1; i += 3) {
389+
const sha = (fields[i] ?? "").replace(/^\n+/, "").trim();
390+
const subject = fields[i + 1] ?? "";
391+
const author = fields[i + 2] ?? "";
392+
if (!sha || !subject.trim()) continue;
393+
commits.push({ sha, subject, author });
394+
}
395+
return commits;
396+
}
397+
180398
export function hasNonWhitespace(text: string): boolean {
181399
return text.replace(/\s+/g, "").length > 0;
182400
}
@@ -425,8 +643,6 @@ export function groupPrsByScope(prs: ReleaseNotePr[]): Array<{ scope: string | n
425643
return groups;
426644
}
427645

428-
const RENDER_CATEGORY_ORDER = ["New Features", "Bug Fixes", "Documentation", "Chores", "Other Changes"];
429-
430646
/**
431647
* Render OpenAI-Codex-style release notes from the generate-notes pieces:
432648
* H2 category sections with scope-grouped, prefix-free summary bullets, then a
@@ -439,6 +655,12 @@ export function renderReleaseNotes(input: {
439655
npmMetadata: string;
440656
carriedPreviewNotes?: string;
441657
deltaPrNotes?: string;
658+
/**
659+
* Pre-rendered category sections for commit-based entries (no PR numbers).
660+
* Used only when the PR pipeline yields nothing, so a release body can never
661+
* collapse to the npm line plus a compare link.
662+
*/
663+
commitFallbackNotes?: string;
442664
compareFrom?: string | null;
443665
compareTo?: string;
444666
repository?: string;
@@ -494,6 +716,20 @@ export function renderReleaseNotes(input: {
494716
parts.push(lines.join("\n"));
495717
}
496718

719+
// Commit fallback: only when the PR pipeline produced no category content at
720+
// all. Its sections are already rendered, so they are appended verbatim.
721+
const renderedAnyPrSection = parts.length > (npmMetadata ? 1 : 0);
722+
if (!renderedAnyPrSection) {
723+
// Carried commit bullets first (older preview work), then this range's own.
724+
// They are merged BY CATEGORY: concatenating two rendered bodies would emit
725+
// `## Bug Fixes` twice when both halves touched the same category.
726+
const merged = mergeCommitBulletSections([
727+
extractCommitBulletSections(input.carriedPreviewNotes ?? ""),
728+
input.commitFallbackNotes ?? "",
729+
]);
730+
if (merged) parts.push(merged);
731+
}
732+
497733
const allPrs = [...categories.values()].flat().sort((a, b) => a.number - b.number);
498734
const from = input.compareFrom?.trim();
499735
const to = input.compareTo?.trim();
@@ -734,6 +970,13 @@ async function main(argv: string[]): Promise<void> {
734970
process.exit(hasMeaningfulCarriedNotes(stripped) ? 0 : 1);
735971
}
736972

973+
if (cmd === "commit-fallback") {
974+
// stdin: `git log --format=%H%x1f%s%x1f%an <range>` output.
975+
const rendered = renderCommitFallbackNotes(parseCommitLog(await readStdinOrFile(rest[0])));
976+
process.stdout.write(rendered);
977+
return;
978+
}
979+
737980
if (cmd === "join-carried") {
738981
let out: string | undefined;
739982
const files: string[] = [];
@@ -888,6 +1131,7 @@ async function main(argv: string[]): Promise<void> {
8881131
"out",
8891132
"carried",
8901133
"delta",
1134+
"commit-fallback",
8911135
"compare-from",
8921136
"compare-to",
8931137
"repository",
@@ -909,6 +1153,7 @@ async function main(argv: string[]): Promise<void> {
9091153
npmMetadata,
9101154
carriedPreviewNotes: await readOptional("carried"),
9111155
deltaPrNotes: await readOptional("delta"),
1156+
commitFallbackNotes: await readOptional("commit-fallback"),
9121157
compareFrom: args.get("compare-from") ?? null,
9131158
compareTo: args.get("compare-to"),
9141159
repository: args.get("repository"),
@@ -972,6 +1217,7 @@ async function main(argv: string[]): Promise<void> {
9721217
Usage:
9731218
bun scripts/release-notes.ts strip-carried [body-file]
9741219
bun scripts/release-notes.ts has-meaningful [body-file]
1220+
bun scripts/release-notes.ts commit-fallback [commit-log-file]
9751221
bun scripts/release-notes.ts join-carried --out <file> <part-file>...
9761222
bun scripts/release-notes.ts matching-preview-tag <version> # tags on stdin
9771223
bun scripts/release-notes.ts matching-preview-tags <version> # tags on stdin, oldest→newest

0 commit comments

Comments
 (0)