From 5eb843365c9064d441a729f0d2d8e5e4f5f7ee7a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 15 Aug 2026 19:23:34 +0900 Subject: [PATCH 1/5] 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. --- .github/workflows/release.yml | 25 +++++++ scripts/release-notes.ts | 123 +++++++++++++++++++++++++++++++++- tests/release-notes.test.ts | 108 +++++++++++++++++++++++++++++ 3 files changed, 254 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1ba8cbd998..3abcc59ca8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -374,8 +374,10 @@ jobs: notes_file="$(mktemp)" carried_file="$(mktemp)" delta_file="$(mktemp)" + commit_fallback_file="$(mktemp)" : > "$carried_file" : > "$delta_file" + : > "$commit_fallback_file" # Stable releases after matching previews: aggregate every matching preview # changelog (oldest→newest; each preview body is incremental), then only @@ -449,6 +451,28 @@ jobs: pr_notes="$(gh api "${generate_notes_api[@]}" --jq '.body')" # Drop generate-notes' trailing compare link; we re-append it after the commit list. printf '%s\n' "$pr_notes" | sed '/^\*\*Full Changelog\*\*:/d' > "$delta_file" + + # generate-notes counts MERGED PULL REQUESTS in the tag range. Work that lands + # as direct commits on the integration branch (or via PRs based on `dev` rather + # than this release branch) leaves that range with nothing to aggregate, and the + # body collapses to the npm line plus a compare link — v2.17.0..v2.18.2 shipped + # exactly that, 0 of 36 commits PR-associated. Fall back to the commit log so a + # release can never publish an empty changelog. + # The renderer only keeps entries carrying a PR number, so the fallback + # travels in its own channel (--commit-fallback) and is emitted only when + # the PR pipeline produced no category sections at all. + if ! bun scripts/release-notes.ts has-meaningful "$delta_file" \ + && ! bun scripts/release-notes.ts has-meaningful "$carried_file"; then + commit_log_file="$(mktemp)" + git log --format='%H%x1f%s%x1f%an' "${notes_range_start}..${GITHUB_SHA}" > "$commit_log_file" + bun scripts/release-notes.ts commit-fallback "$commit_log_file" > "$commit_fallback_file" + if bun scripts/release-notes.ts has-meaningful "$commit_fallback_file"; then + echo "::notice::generate-notes returned no PR categories for ${notes_range_start}..${release_tag}; using the commit-based changelog fallback" + else + : > "$commit_fallback_file" + echo "::notice::No PR categories and no eligible commits in ${notes_range_start}..${release_tag}; release notes stay minimal" + fi + fi else # First release on this channel: never call generate-notes without previous_tag_name. # GitHub would baseline the newest repo tag, which may belong to the other channel. @@ -476,6 +500,7 @@ jobs: --npm-metadata "$npm_metadata" --carried "$carried_file" --delta "$delta_file" + --commit-fallback "$commit_fallback_file" --out "$notes_file" --compare-to "$release_tag" --repository "$GITHUB_REPOSITORY" diff --git a/scripts/release-notes.ts b/scripts/release-notes.ts index 98aa308731..0dfe0530af 100644 --- a/scripts/release-notes.ts +++ b/scripts/release-notes.ts @@ -10,6 +10,7 @@ * bun scripts/release-notes.ts matching-preview-tags * bun scripts/release-notes.ts previous-release-tag * bun scripts/release-notes.ts has-meaningful [body-file] + * bun scripts/release-notes.ts commit-fallback [commit-log-file] * bun scripts/release-notes.ts credit-takeovers --repo --in --out * bun scripts/release-notes.ts render --npm-metadata ... --out ... [--carried ...] [--delta ...] [--compare-from ...] [--compare-to ...] [--repository ...] * bun scripts/release-notes.ts polish --in --out [--model ...] [--base-url ...] @@ -177,6 +178,104 @@ export function hasMeaningfulCarriedNotes(stripped: string): boolean { return !isEmptyGeneratedNotes(stripped); } +/** + * A single commit considered for the commit-based changelog fallback. + * `sha` is the full or short hash; `subject` is the commit subject line. + */ +export type ReleaseNoteCommit = { + sha: string; + subject: string; + author: string; +}; + +/** Category order shared by the PR renderer and the commit fallback. */ +const RENDER_CATEGORY_ORDER = ["New Features", "Bug Fixes", "Documentation", "Chores", "Other Changes"]; + +/** Conventional-commit type -> release.yml category title. */ +const COMMIT_TYPE_CATEGORY: Record = { + feat: "New Features", + fix: "Bug Fixes", + perf: "Bug Fixes", + docs: "Documentation", + chore: "Chores", + build: "Chores", + ci: "Chores", + refactor: "Chores", + style: "Chores", + test: "Chores", +}; + +/** + * Commits that are release plumbing rather than shipped work. A merge commit's + * content is already represented by the commits it brings in, and a `release:` + * bump is the release itself. + */ +export function isReleasePlumbingCommit(subject: string): boolean { + const text = subject.trim(); + if (/^Merge\s/i.test(text)) return true; + if (/^release:\s/i.test(text)) return true; + return false; +} + +/** + * Render commits as a generate-notes-shaped body so the existing category + * parser/renderer can consume them unchanged. + * + * Why this exists: `releases/generate-notes` aggregates MERGED PULL REQUESTS + * against the compared tag range. When work lands as direct commits on the + * integration branch (or through PRs whose base is `dev` rather than the + * release branch), that range contains no PRs the API will count and the body + * collapses to the npm line plus a compare link — v2.17.0..v2.18.2 had 0 of 36 + * commits associated with a main-merged PR, and both releases shipped an empty + * changelog. The fallback keeps the release body honest regardless of how the + * work reached the branch. + * + * Commits carry no PR number, so the synthetic entries use `#0` — a sentinel + * the renderer never prints as a link because these are emitted as plain + * bullets under their category heading. + */ +export function renderCommitFallbackNotes(commits: ReleaseNoteCommit[]): string { + const buckets = new Map(); + for (const commit of commits) { + const subject = commit.subject.trim(); + if (!subject) continue; + if (isReleasePlumbingCommit(subject)) continue; + const match = /^([a-zA-Z]+)(?:\(([^)]*)\))?!?:\s*(.+)$/.exec(subject); + const type = match?.[1]?.toLowerCase(); + const scope = match?.[2]?.trim(); + const summary = (match?.[3] ?? subject).trim(); + const category = (type && COMMIT_TYPE_CATEGORY[type]) ?? "Other Changes"; + const shortSha = commit.sha.trim().slice(0, 9); + const scopePrefix = scope ? `${scope}: ` : ""; + const author = commit.author.trim(); + const credit = author ? ` @${author}` : ""; + const line = `- ${scopePrefix}${summary} (${shortSha})${credit}`; + const existing = buckets.get(category); + if (existing) existing.push(line); + else buckets.set(category, [line]); + } + if (buckets.size === 0) return ""; + const parts: string[] = []; + for (const title of RENDER_CATEGORY_ORDER) { + const lines = buckets.get(title); + if (!lines || lines.length === 0) continue; + parts.push([`## ${title}`, "", ...lines].join("\n")); + } + return parts.join("\n\n").replace(/\n+$/, "") + "\n"; +} + +/** Parse `git log --format=%H%x1f%s%x1f%an` output into commits. */ +export function parseCommitLog(raw: string): ReleaseNoteCommit[] { + const commits: ReleaseNoteCommit[] = []; + for (const line of raw.replace(/\r\n/g, "\n").split("\n")) { + if (!line.trim()) continue; + const [sha, subject, author] = line.split("\u001f"); + if (!sha || !subject) continue; + commits.push({ sha, subject, author: author ?? "" }); + } + return commits; +} + export function hasNonWhitespace(text: string): boolean { return text.replace(/\s+/g, "").length > 0; } @@ -425,8 +524,6 @@ export function groupPrsByScope(prs: ReleaseNotePr[]): Array<{ scope: string | n return groups; } -const RENDER_CATEGORY_ORDER = ["New Features", "Bug Fixes", "Documentation", "Chores", "Other Changes"]; - /** * Render OpenAI-Codex-style release notes from the generate-notes pieces: * H2 category sections with scope-grouped, prefix-free summary bullets, then a @@ -439,6 +536,12 @@ export function renderReleaseNotes(input: { npmMetadata: string; carriedPreviewNotes?: string; deltaPrNotes?: string; + /** + * Pre-rendered category sections for commit-based entries (no PR numbers). + * Used only when the PR pipeline yields nothing, so a release body can never + * collapse to the npm line plus a compare link. + */ + commitFallbackNotes?: string; compareFrom?: string | null; compareTo?: string; repository?: string; @@ -494,6 +597,12 @@ export function renderReleaseNotes(input: { parts.push(lines.join("\n")); } + // Commit fallback: only when the PR pipeline produced no category content at + // all. Its sections are already rendered, so they are appended verbatim. + const renderedAnyPrSection = parts.length > (npmMetadata ? 1 : 0); + const commitFallback = (input.commitFallbackNotes ?? "").trim(); + if (!renderedAnyPrSection && commitFallback) parts.push(commitFallback); + const allPrs = [...categories.values()].flat().sort((a, b) => a.number - b.number); const from = input.compareFrom?.trim(); const to = input.compareTo?.trim(); @@ -734,6 +843,13 @@ async function main(argv: string[]): Promise { process.exit(hasMeaningfulCarriedNotes(stripped) ? 0 : 1); } + if (cmd === "commit-fallback") { + // stdin: `git log --format=%H%x1f%s%x1f%an ` output. + const rendered = renderCommitFallbackNotes(parseCommitLog(await readStdinOrFile(rest[0]))); + process.stdout.write(rendered); + return; + } + if (cmd === "join-carried") { let out: string | undefined; const files: string[] = []; @@ -888,6 +1004,7 @@ async function main(argv: string[]): Promise { "out", "carried", "delta", + "commit-fallback", "compare-from", "compare-to", "repository", @@ -909,6 +1026,7 @@ async function main(argv: string[]): Promise { npmMetadata, carriedPreviewNotes: await readOptional("carried"), deltaPrNotes: await readOptional("delta"), + commitFallbackNotes: await readOptional("commit-fallback"), compareFrom: args.get("compare-from") ?? null, compareTo: args.get("compare-to"), repository: args.get("repository"), @@ -972,6 +1090,7 @@ async function main(argv: string[]): Promise { Usage: bun scripts/release-notes.ts strip-carried [body-file] bun scripts/release-notes.ts has-meaningful [body-file] + bun scripts/release-notes.ts commit-fallback [commit-log-file] bun scripts/release-notes.ts join-carried --out ... bun scripts/release-notes.ts matching-preview-tag # tags on stdin bun scripts/release-notes.ts matching-preview-tags # tags on stdin, oldest→newest diff --git a/tests/release-notes.test.ts b/tests/release-notes.test.ts index e9a0562c82..bb89ebed16 100644 --- a/tests/release-notes.test.ts +++ b/tests/release-notes.test.ts @@ -4,14 +4,17 @@ import { extractChangelogPrNumbers, extractPrNumbers, hasMeaningfulCarriedNotes, + isReleasePlumbingCommit, isPolishBaseUrlAllowed, joinCarriedPreviewNotes, matchingPreviewTag, matchingPreviewTags, + parseCommitLog, parseGeneratedNotes, parseSectionHeadings, parseTakeoverSourcePr, previousReleaseNotesTag, + renderCommitFallbackNotes, renderReleaseNotes, rewriteTakeoverCredits, selectNewestCarriedPreviewTag, @@ -696,3 +699,108 @@ describe("polish validation", () => { expect(errors).toContain("unexpected headings: Internal"); }); }); + +describe("commit-based changelog fallback", () => { + const log = [ + "aaaaaaaaaaaa1\u001ffeat(gui): add a quota badge\u001falice", + "bbbbbbbbbbbb2\u001ffix(codex): stop a launcher crash (#1625)\u001fbob", + "cccccccccccc3\u001fdocs(devlog): record the release train\u001fcarol", + "dddddddddddd4\u001fchore(ci): prune stale workflows\u001fdave", + "eeeeeeeeeeee5\u001fjust a bare subject\u001feve", + "ffffffffffff6\u001fMerge dev into main: v9.9.9 release\u001fmallory", + "gggggggggggg7\u001frelease: v9.9.9\u001ftrent", + ].join("\n"); + + test("parseCommitLog reads the unit-separated git log format", () => { + const commits = parseCommitLog(log); + expect(commits).toHaveLength(7); + expect(commits[0]).toEqual({ sha: "aaaaaaaaaaaa1", subject: "feat(gui): add a quota badge", author: "alice" }); + }); + + test("parseCommitLog ignores blank and malformed lines", () => { + expect(parseCommitLog("")).toEqual([]); + expect(parseCommitLog("\n\n")).toEqual([]); + // A hash with no subject carries no changelog value. + expect(parseCommitLog("abc123")).toEqual([]); + }); + + test("conventional prefixes map onto the release.yml categories", () => { + const out = renderCommitFallbackNotes(parseCommitLog(log)); + expect(out).toContain("## New Features"); + expect(out).toContain("- gui: add a quota badge (aaaaaaaaa) @alice"); + expect(out).toContain("## Bug Fixes"); + expect(out).toContain("- codex: stop a launcher crash (#1625) (bbbbbbbbb) @bob"); + expect(out).toContain("## Documentation"); + expect(out).toContain("## Chores"); + expect(out).toContain("## Other Changes"); + expect(out).toContain("- just a bare subject (eeeeeeeee) @eve"); + }); + + test("categories render in the canonical order", () => { + const out = renderCommitFallbackNotes(parseCommitLog(log)); + const order = ["## New Features", "## Bug Fixes", "## Documentation", "## Chores", "## Other Changes"] + .map(heading => out.indexOf(heading)); + expect(order).toEqual([...order].sort((a, b) => a - b)); + expect(order.every(index => index >= 0)).toBe(true); + }); + + test("merge commits and release bumps are excluded", () => { + const out = renderCommitFallbackNotes(parseCommitLog(log)); + expect(out).not.toContain("Merge dev into main"); + expect(out).not.toContain("release: v9.9.9"); + expect(isReleasePlumbingCommit("Merge pull request #1 from x/y")).toBe(true); + expect(isReleasePlumbingCommit("release: v2.20.0")).toBe(true); + expect(isReleasePlumbingCommit("fix(codex): a real fix")).toBe(false); + }); + + test("a range with only plumbing commits renders nothing, so the caller keeps minimal notes", () => { + const plumbingOnly = [ + "ffffffffffff6\u001fMerge dev into main: v9.9.9 release\u001fmallory", + "gggggggggggg7\u001frelease: v9.9.9\u001ftrent", + ].join("\n"); + const out = renderCommitFallbackNotes(parseCommitLog(plumbingOnly)); + expect(out).toBe(""); + expect(hasMeaningfulCarriedNotes(out)).toBe(false); + }); + + test("no commits at all renders nothing", () => { + expect(renderCommitFallbackNotes([])).toBe(""); + }); + + test("real PR sections win; the commit fallback is not appended alongside them", () => { + const rendered = renderReleaseNotes({ + npmMetadata: "npm line.", + deltaPrNotes: "## Bug Fixes\n\n* fix a thing by @dev in https://github.com/o/n/pull/42\n", + commitFallbackNotes: renderCommitFallbackNotes(parseCommitLog(log)), + compareFrom: "v9.9.8", + compareTo: "v9.9.9", + repository: "owner/name", + }); + expect(rendered).toContain("#42"); + expect(rendered).not.toContain("@alice"); + expect(rendered).not.toContain("aaaaaaaaa"); + }); + + test("the fallback output is meaningful, which is the workflow's switch condition", () => { + // The workflow calls has-meaningful on the generate-notes body; when that is + // empty it renders the commit fallback and checks the same predicate again. + const emptyGenerateNotes = "\n\n\n"; + expect(hasMeaningfulCarriedNotes(emptyGenerateNotes)).toBe(false); + expect(hasMeaningfulCarriedNotes(renderCommitFallbackNotes(parseCommitLog(log)))).toBe(true); + }); + + test("fallback sections flow through the real renderer into a non-empty body", () => { + // The whole point: an empty generate-notes delta must still produce a body + // with categorized content rather than the npm line plus a compare link. + const rendered = renderReleaseNotes({ + npmMetadata: "Published to npm as \`pkg@9.9.9\` with dist-tag \`latest\`.", + commitFallbackNotes: renderCommitFallbackNotes(parseCommitLog(log)), + compareFrom: "v9.9.8", + compareTo: "v9.9.9", + repository: "owner/name", + }); + expect(rendered).toContain("## New Features"); + expect(rendered).toContain("## Bug Fixes"); + expect(rendered.length).toBeGreaterThan(400); + }); +}); From a2be38db2591e06df33abeee4726525a45d56ea8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 15 Aug 2026 19:30:34 +0900 Subject: [PATCH 2/5] 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. --- scripts/release-notes.ts | 109 +++++++++++++++++++++++++++++++----- tests/release-notes.test.ts | 105 +++++++++++++++++++++++++++++++++- 2 files changed, 198 insertions(+), 16 deletions(-) diff --git a/scripts/release-notes.ts b/scripts/release-notes.ts index 0dfe0530af..b85202cc45 100644 --- a/scripts/release-notes.ts +++ b/scripts/release-notes.ts @@ -213,10 +213,31 @@ const COMMIT_TYPE_CATEGORY: Record = { export function isReleasePlumbingCommit(subject: string): boolean { const text = subject.trim(); if (/^Merge\s/i.test(text)) return true; - if (/^release:\s/i.test(text)) return true; + // Real two-parent merges in this repo also use a `merge:` conventional prefix. + if (/^merge(?:\([^)]*\))?!?:\s/i.test(text)) return true; + if (/^release(?:\([^)]*\))?!?:\s/i.test(text)) return true; return false; } +/** + * Neutralize Markdown and mention syntax from untrusted commit text before it + * lands in a release body. Commit subjects and author names are attacker- or + * accident-controlled: a bare `@name` renders as a real GitHub mention (and + * notifies that account), and backticks/brackets can restructure the notes. + */ +export function sanitizeCommitText(text: string): string { + return text + .replace(/\r?\n/g, " ") + // Strip the ASCII unit separator so a subject can never forge a log field. + .replace(/\u001f/g, " ") + .replace(/[`<>|]/g, "") + .replace(/([[\]])/g, "\\$1") + // `@name` -> `@\u200bname`: reads identically, never notifies. + .replace(/@(?=[A-Za-z0-9_-])/g, "@\u200b") + .replace(/\s+/g, " ") + .trim(); +} + /** * Render commits as a generate-notes-shaped body so the existing category * parser/renderer can consume them unchanged. @@ -242,14 +263,21 @@ export function renderCommitFallbackNotes(commits: ReleaseNoteCommit[]): string if (isReleasePlumbingCommit(subject)) continue; const match = /^([a-zA-Z]+)(?:\(([^)]*)\))?!?:\s*(.+)$/.exec(subject); const type = match?.[1]?.toLowerCase(); - const scope = match?.[2]?.trim(); - const summary = (match?.[3] ?? subject).trim(); + const scope = sanitizeCommitText(match?.[2] ?? ""); + const summary = sanitizeCommitText(match?.[3] ?? subject); + if (!summary) continue; const category = (type && COMMIT_TYPE_CATEGORY[type]) ?? "Other Changes"; - const shortSha = commit.sha.trim().slice(0, 9); + // Hex-only short hash: a crafted `sha` field can never inject markup. + const shortSha = /^[0-9a-f]{7,40}$/i.test(commit.sha.trim()) + ? commit.sha.trim().slice(0, 9) + : ""; const scopePrefix = scope ? `${scope}: ` : ""; - const author = commit.author.trim(); - const credit = author ? ` @${author}` : ""; - const line = `- ${scopePrefix}${summary} (${shortSha})${credit}`; + // `%an` is a free-form Git display name, not a GitHub login, so it is + // rendered as plain text rather than an @mention that would notify a + // same-named (or non-existent) account. + const author = sanitizeCommitText(commit.author).replace(/^@\u200b/, ""); + const trailer = [shortSha, author].filter(Boolean).join(", "); + const line = trailer ? `- ${scopePrefix}${summary} (${trailer})` : `- ${scopePrefix}${summary}`; const existing = buckets.get(category); if (existing) existing.push(line); else buckets.set(category, [line]); @@ -264,14 +292,64 @@ export function renderCommitFallbackNotes(commits: ReleaseNoteCommit[]): string return parts.join("\n\n").replace(/\n+$/, "") + "\n"; } -/** Parse `git log --format=%H%x1f%s%x1f%an` output into commits. */ +/** + * Extract commit-style category sections (bullets with no `(#N)` reference) + * from an already-rendered body. + * + * A preview release whose notes came from the commit fallback carries bullets + * like `- gui: fix a thing (abc1234, Name)`. Those are meaningful prose, so the + * workflow keeps them as carried notes and skips regenerating a fallback — but + * the PR renderer only retains entries carrying a PR number, so without this + * the stable release would silently collapse back to the npm-line stub. + */ +export function extractCommitBulletSections(body: string): string { + const out: string[] = []; + let current: { title: string; lines: string[] } | null = null; + const flush = (): void => { + if (current && current.lines.length > 0) { + out.push([`## ${current.title}`, "", ...current.lines].join("\n")); + } + current = null; + }; + for (const rawLine of body.replace(/\r\n/g, "\n").split("\n")) { + const line = rawLine.trim(); + if (!line || line.startsWith("