From 9a356aed4fd5e47323da55e32aeb7c53c7d0c76e Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:53:31 +0700 Subject: [PATCH 1/3] ci(ops): merge-when-green writes a commitlint-clean squash message (#1130) GitHub's default squash message keeps each commit's long lines and mixes the co-author trailers into the body, so the head of dev could fail the commitlint check of the next promotion PR (DOS/Rate#1128). The script now sends commit_title (the PR title with its number, cut at a word if it would pass 100 characters) and commit_message (each commit as a list item, wrapped at 100 characters, trailers gathered once into a footer). Co-Authored-By: Claude Opus 5.5 --- Docs/Resources/RELEASE.md | 2 +- scripts/ops/merge-when-green.mjs | 119 ++++++++++++++++++++++++++ scripts/ops/merge-when-green.test.mjs | 84 ++++++++++++++++++ 3 files changed, 204 insertions(+), 1 deletion(-) diff --git a/Docs/Resources/RELEASE.md b/Docs/Resources/RELEASE.md index fcfb5c3d..8fdc80d4 100644 --- a/Docs/Resources/RELEASE.md +++ b/Docs/Resources/RELEASE.md @@ -11,7 +11,7 @@ This is the release workflow for `rate.vn`. It follows the same model as DOS/DOS ## Normal release -1. Feature PRs go into `dev` as usual: review, green required checks (`Typecheck & Build Monorepo`, `E2E (Playwright)`, `Secret scan (gitleaks)`, `task-claim`), squash merge. +1. Feature PRs go into `dev` as usual: review, green required checks (`Typecheck & Build Monorepo`, `E2E (Playwright)`, `Secret scan (gitleaks)`, `task-claim`), squash merge. `node scripts/ops/merge-when-green.mjs ` writes the squash commit's message itself: the PR title with `(#)`, then each commit wrapped at 100 characters, with the co-author trailers in a footer. The promotion PR's `commitlint` check lints the head of `dev`, which is usually that squash commit, so it must pass `commitlint.config.ts` (DOS/Rate#1130). 2. Check `dev` on its Vercel Preview deployment. 3. Open the promotion PR from `dev` itself, not from a copy branch: `gh pr create --repo DOS/Rate --base main --head dev --title "chore(release): promote dev to main"` diff --git a/scripts/ops/merge-when-green.mjs b/scripts/ops/merge-when-green.mjs index e1f25822..09c05aee 100644 --- a/scripts/ops/merge-when-green.mjs +++ b/scripts/ops/merge-when-green.mjs @@ -131,6 +131,99 @@ export function shouldDeleteBranch({ method, headRef, headRepo, baseRepo }) { ); } +/** + * commitlint's limit (commitlint.config.ts) for the header and for every body + * and footer line. The commitlint check of a dev to main promotion lints the + * head of dev, which is usually a squash commit made here (DOS/Rate#1130). + */ +export const MAX_LINE = 100; +// commitlint exempts any line holding a URL from the line limit. +const URL_LINE = /\bhttps?:\/\/\S+/; +const TRAILER = /^(?:co-authored-by|signed-off-by):\s*\S/i; +const LIST_MARKER = /^\s*(?:[-*+]\s+|\d+[.)]\s+)?/; + +/** + * One line as lines of at most `width` characters: words wrap under the text + * of a list item, and a word longer than a whole line is split. + */ +export function wrapLine(line, width = MAX_LINE) { + if (line.length <= width || URL_LINE.test(line)) return [line]; + const marker = line.match(LIST_MARKER)[0]; + const first = marker.length <= width / 2 ? marker : ''; + const hang = ' '.repeat(first.length); + const lines = []; + let current = first; + let empty = true; + for (let word of line.slice(first.length).split(/\s+/).filter(Boolean)) { + for (;;) { + const sep = empty ? '' : ' '; + if (current.length + sep.length + word.length <= width) { + current += sep + word; + empty = false; + break; + } + if (empty) { + // Not even an empty line holds the word: split it. + const room = width - current.length; + lines.push(current + word.slice(0, room)); + word = word.slice(room); + current = hang; + continue; + } + lines.push(current); + current = hang; + empty = true; + } + } + if (!empty) lines.push(current); + return lines; +} + +/** + * The squash commit's header: the PR title with its number, which promote.mjs + * reads back. A title too long for both is cut at a word. + */ +export function squashTitle(title, number) { + const suffix = ` (#${number})`; + const text = title.trim().replace(/\s+/g, ' '); + if (text.length + suffix.length <= MAX_LINE) return text + suffix; + const room = MAX_LINE - suffix.length; + const cut = text.slice(0, room + 1).replace(/\s+\S*$/, ''); + const head = ( + cut.length > 0 && cut.length <= room ? cut : text.slice(0, room) + ).replace(/[\s.,;:!?-]+$/, ''); + return head + suffix; +} + +/** + * The squash commit's body, like GitHub's default (each commit as a list + * item), with every line wrapped to the limit and the commits' co-author and + * sign-off trailers gathered once each into a footer after a blank line. + */ +export function squashBody(messages) { + const trailers = new Map(); + const items = []; + for (const message of messages) { + const lines = []; + for (const line of message.replace(/\r\n?/g, '\n').split('\n')) { + if (TRAILER.test(line.trim())) { + const trailer = line.trim().replace(/:\s*/, ': '); + const key = trailer.toLowerCase(); + if (!trailers.has(key)) trailers.set(key, trailer); + } else { + lines.push(line.trimEnd()); + } + } + while (lines.length > 0 && lines.at(-1) === '') lines.pop(); + while (lines.length > 0 && lines[0] === '') lines.shift(); + if (lines.length === 0) continue; + lines[0] = `* ${lines[0]}`; + items.push(lines.flatMap((line) => wrapLine(line)).join('\n')); + } + const footer = [...trailers.values()].flatMap((line) => wrapLine(line)); + return [items.join('\n\n'), footer.join('\n')].filter(Boolean).join('\n\n'); +} + /** * What to do with the PR now: 'merge', 'wait', or a final 'done' / 'fail' * with a one-line message. @@ -210,6 +303,7 @@ export async function mergeWhenGreen({ let blockedPolls = 0; let apiErrors = 0; let mergeRefusals = 0; + let commitListErrors = 0; let branchUpdates = 0; let lastUpdatedSha = null; for (;;) { @@ -293,6 +387,30 @@ export async function mergeWhenGreen({ continue; } if (next.action === 'merge') { + // GitHub's default squash message keeps the commits' long lines and + // mixes their trailers into the body: send one commitlint accepts. + const squash = []; + if (method === 'squash') { + let messages; + try { + messages = ghPaginated( + `repos/${repo}/pulls/${number}/commits?per_page=100`, + '.[] | {message: .commit.message}' + ).map((c) => c.message); + } catch (err) { + if (++commitListErrors >= MAX_API_ERRORS) { + return { ok: false, message: `PR ${number}: ${err.message}` }; + } + await sleep(intervalMs); + continue; + } + squash.push( + '-f', + `commit_title=${squashTitle(pr.title, number)}`, + '-f', + `commit_message=${squashBody(messages)}` + ); + } const merged = gh( [ 'api', @@ -303,6 +421,7 @@ export async function mergeWhenGreen({ `merge_method=${method}`, '-f', `sha=${pr.head.sha}`, + ...squash, '--jq', '.sha', ], diff --git a/scripts/ops/merge-when-green.test.mjs b/scripts/ops/merge-when-green.test.mjs index 3329c13a..781c17f5 100644 --- a/scripts/ops/merge-when-green.test.mjs +++ b/scripts/ops/merge-when-green.test.mjs @@ -1,4 +1,6 @@ import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; import test from 'node:test'; import { @@ -7,9 +9,13 @@ import { evaluateChecks, ignoredRequired, latestRuns, + MAX_LINE, mergeMethodFor, requiredChecksFromRules, shouldDeleteBranch, + squashBody, + squashTitle, + wrapLine, } from './merge-when-green.mjs'; import { encodeRef } from './lib.mjs'; @@ -291,3 +297,81 @@ test('gives a green but blocked PR a few polls before reporting it', () => { assert.equal(last.action, 'fail'); assert.match(last.message, /required review or check/); }); + +// The commits of DOS/Rate#1127, whose default squash failed commitlint on the +// promotion PR DOS/Rate#1128 (DOS/Rate#1130). +const CO_AUTHOR = 'Co-Authored-By: Claude Opus 5.5 '; +const LONG_COMMITS = [ + `fix(shared-data): earbuds regex reads glued brands and drops over-ear titles that say tai nghe (#1125) + +Any brand glued to Buds, FreeBuds, AirPods or WF1000XM now reads as earbuds +(PixelBudsPro2, HuaweiFreeBudsPro4, SonyWF1000XM5), as do AirPodsPro2 and +two glued tiers (Buds3ProFE). + +${CO_AUTHOR}`, + `fix(shared-data): over-ear WH with a space or underscore, glued Redmi/Samsung/Enco/Tune Buds (#1125) + +Review follow-up on PR #1127, with a reference line that is far longer than one hundred characters in total. +See https://github.com/DOS/Rate/pull/1127#issuecomment-1234567890 for the review thread and the whole discussion. +A path with no spaces: ${'packages/shared-data/src/'.repeat(6)}earbuds.ts + +${CO_AUTHOR}`, +]; +const LONG_TITLE = + 'fix(shared-data): earbuds regex reads glued brands, drops over-ear tai nghe titles (#1125)'; + +const repoRoot = fileURLToPath(new URL('../../', import.meta.url)); +/** Lints a message with the repository's commitlint.config.ts. */ +function commitlint(message) { + const r = spawnSync( + process.execPath, + ['node_modules/@commitlint/cli/cli.js'], + { cwd: repoRoot, input: message, encoding: 'utf8' } + ); + assert.equal(r.error, undefined); + return { ok: r.status === 0, output: r.stdout + r.stderr }; +} + +test('wraps a long line under its list marker and splits a word longer than a line', () => { + const words = Array.from({ length: 40 }, (_, i) => `word${i}`).join(' '); + const wrapped = wrapLine(`* ${words}`); + assert.ok(wrapped.length > 1); + assert.ok(wrapped.every((line) => line.length <= MAX_LINE)); + assert.ok(wrapped.slice(1).every((line) => line.startsWith(' w'))); + assert.equal(wrapped.join(' ').replace(/\s+/g, ' '), `* ${words}`); + const word = 'x'.repeat(250); + assert.equal(wrapLine(word).join(''), word); + assert.ok(wrapLine(word).every((line) => line.length <= MAX_LINE)); + assert.deepEqual(wrapLine('short'), ['short']); + const url = `see https://example.com/${'a'.repeat(120)}`; + assert.deepEqual(wrapLine(url), [url]); +}); + +test('keeps the PR number in the squash title and cuts a long title at a word', () => { + assert.equal(squashTitle('fix(web): a thing', 7), 'fix(web): a thing (#7)'); + const title = squashTitle(LONG_TITLE, 1127); + assert.equal(title, `${LONG_TITLE} (#1127)`); + const longer = squashTitle(`${LONG_TITLE} and more words here`, 1127); + assert.ok(longer.length <= MAX_LINE); + assert.equal(longer, `${LONG_TITLE} (#1127)`); +}); + +test('gathers the trailers once each into a footer after the commits', () => { + const body = squashBody(LONG_COMMITS); + assert.ok(body.startsWith('* fix(shared-data): earbuds regex')); + assert.ok(body.endsWith(`\n\n${CO_AUTHOR}`)); + assert.equal(body.split(CO_AUTHOR).length, 2); + assert.ok(body.includes('\n\n* fix(shared-data): over-ear WH')); + assert.equal(squashBody(['fix: a\r\n\r\nbody\r\n']), '* fix: a\n\nbody'); +}); + +test('the squash message of PR bodies with lines over 100 characters passes commitlint', () => { + const header = squashTitle(LONG_TITLE, 1127); + const message = `${header}\n\n${squashBody(LONG_COMMITS)}\n`; + // GitHub's default squash of the same commits fails, so the test has teeth. + const github = `${header}\n\n${LONG_COMMITS.map((m) => `* ${m}`).join('\n\n')}\n`; + assert.equal(commitlint(github).ok, false); + const result = commitlint(message); + assert.ok(result.ok, result.output); + assert.doesNotMatch(result.output, /warning/); +}); From c17ab01f731e3713c28ac9a9d5bc67757696e1c0 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:02:52 +0700 Subject: [PATCH 2/3] ci(ops): refuse unfit titles, send the merge as JSON on stdin (#1130) Review follow-up on PR #1133. A title that is not a conventional header or does not fit " (#n)" in 100 characters is refused before waiting, instead of being cut into an empty or start-case subject. The merge fields go to gh on stdin, so a long squash message is not over the Windows command line limit. Long words split between characters, a trailer over 100 characters is left out instead of wrapped, a PR whose commits cannot all be listed is refused, and the commit-list retry counter resets after a success. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- scripts/ops/lib.mjs | 3 +- scripts/ops/merge-when-green.mjs | 82 ++++++++++++++++++--------- scripts/ops/merge-when-green.test.mjs | 37 +++++++++--- 3 files changed, 88 insertions(+), 34 deletions(-) diff --git a/scripts/ops/lib.mjs b/scripts/ops/lib.mjs index 297cd4b3..e7c6f4ff 100644 --- a/scripts/ops/lib.mjs +++ b/scripts/ops/lib.mjs @@ -7,9 +7,10 @@ import { pathToFileURL } from 'node:url'; export const DEFAULT_REPO = 'DOS/Rate'; -export function gh(args, { allowFailure = false } = {}) { +export function gh(args, { allowFailure = false, input } = {}) { const r = spawnSync('gh', args, { encoding: 'utf8', + input, maxBuffer: 64 * 1024 * 1024, }); if (r.error) throw r.error; diff --git a/scripts/ops/merge-when-green.mjs b/scripts/ops/merge-when-green.mjs index 09c05aee..93426e0b 100644 --- a/scripts/ops/merge-when-green.mjs +++ b/scripts/ops/merge-when-green.mjs @@ -147,6 +147,7 @@ const LIST_MARKER = /^\s*(?:[-*+]\s+|\d+[.)]\s+)?/; * of a list item, and a word longer than a whole line is split. */ export function wrapLine(line, width = MAX_LINE) { + if (!(width >= 2)) throw new RangeError(`cannot wrap to width ${width}`); if (line.length <= width || URL_LINE.test(line)) return [line]; const marker = line.match(LIST_MARKER)[0]; const first = marker.length <= width / 2 ? marker : ''; @@ -164,9 +165,16 @@ export function wrapLine(line, width = MAX_LINE) { } if (empty) { // Not even an empty line holds the word: split it. + // Cut between characters, never inside a surrogate pair. const room = width - current.length; - lines.push(current + word.slice(0, room)); - word = word.slice(room); + let piece = ''; + for (const char of word) { + if (piece.length + char.length > room) break; + piece += char; + } + if (piece === '') piece = word.slice(0, Math.max(room, 1)); + lines.push(current + piece); + word = word.slice(piece.length); current = hang; continue; } @@ -179,20 +187,20 @@ export function wrapLine(line, width = MAX_LINE) { return lines; } +// `type(scope)!: subject`, the header shape commitlint's parser reads. +const CONVENTIONAL_HEADER = /^[a-z]+(?:\([^()\r\n]*\))?!?: \S/; + /** * The squash commit's header: the PR title with its number, which promote.mjs - * reads back. A title too long for both is cut at a word. + * reads back. Null when the title is not a conventional header or the header + * is over the limit: cutting it could leave an empty or start-case subject. */ export function squashTitle(title, number) { - const suffix = ` (#${number})`; const text = title.trim().replace(/\s+/g, ' '); - if (text.length + suffix.length <= MAX_LINE) return text + suffix; - const room = MAX_LINE - suffix.length; - const cut = text.slice(0, room + 1).replace(/\s+\S*$/, ''); - const head = ( - cut.length > 0 && cut.length <= room ? cut : text.slice(0, room) - ).replace(/[\s.,;:!?-]+$/, ''); - return head + suffix; + const header = `${text} (#${number})`; + return CONVENTIONAL_HEADER.test(text) && header.length <= MAX_LINE + ? header + : null; } /** @@ -220,7 +228,10 @@ export function squashBody(messages) { lines[0] = `* ${lines[0]}`; items.push(lines.flatMap((line) => wrapLine(line)).join('\n')); } - const footer = [...trailers.values()].flatMap((line) => wrapLine(line)); + // A wrapped trailer no longer credits anyone: leave out one too long. + const footer = [...trailers.values()].filter( + (line) => line.length <= MAX_LINE + ); return [items.join('\n\n'), footer.join('\n')].filter(Boolean).join('\n\n'); } @@ -343,6 +354,12 @@ export async function mergeWhenGreen({ message: `PR ${number}: cannot --ignore required check(s) ${conflict.join(', ')}`, }; } + if (method === 'squash' && !squashTitle(pr.title, number)) { + return { + ok: false, + message: `PR ${number}: its title must be a conventional header that fits "<title> (#${number})" in ${MAX_LINE} characters, so the squash commit passes commitlint`, + }; + } } const checks = evaluateChecks(head.runs, head.statuses, { ignore: [...IGNORED_CHECKS, ...ignore], @@ -389,7 +406,7 @@ export async function mergeWhenGreen({ if (next.action === 'merge') { // GitHub's default squash message keeps the commits' long lines and // mixes their trailers into the body: send one commitlint accepts. - const squash = []; + const squash = {}; if (method === 'squash') { let messages; try { @@ -397,35 +414,48 @@ export async function mergeWhenGreen({ `repos/${repo}/pulls/${number}/commits?per_page=100`, '.[] | {message: .commit.message}' ).map((c) => c.message); + commitListErrors = 0; } catch (err) { if (++commitListErrors >= MAX_API_ERRORS) { - return { ok: false, message: `PR ${number}: ${err.message}` }; + return { + ok: false, + message: `PR ${number}: cannot list its commits: ${err.message}`, + }; } await sleep(intervalMs); continue; } - squash.push( - '-f', - `commit_title=${squashTitle(pr.title, number)}`, - '-f', - `commit_message=${squashBody(messages)}` - ); + // The commits API stops at 250: never write a body that drops some. + if (messages.length < pr.commits) { + return { + ok: false, + message: `PR ${number}: only ${messages.length} of its ${pr.commits} commits could be listed for the squash message`, + }; + } + squash.commit_title = squashTitle(pr.title, number); + squash.commit_message = squashBody(messages); } + // The fields go as JSON on stdin: a long squash message is over the + // Windows command line limit. const merged = gh( [ 'api', '-X', 'PUT', `repos/${repo}/pulls/${number}/merge`, - '-f', - `merge_method=${method}`, - '-f', - `sha=${pr.head.sha}`, - ...squash, + '--input', + '-', '--jq', '.sha', ], - { allowFailure: true } + { + allowFailure: true, + input: JSON.stringify({ + merge_method: method, + sha: pr.head.sha, + ...squash, + }), + } ); if (merged.status !== 0) { // A check re-queued after the last read (for example by a PR edit) diff --git a/scripts/ops/merge-when-green.test.mjs b/scripts/ops/merge-when-green.test.mjs index 781c17f5..2000e1f1 100644 --- a/scripts/ops/merge-when-green.test.mjs +++ b/scripts/ops/merge-when-green.test.mjs @@ -347,13 +347,30 @@ test('wraps a long line under its list marker and splits a word longer than a li assert.deepEqual(wrapLine(url), [url]); }); -test('keeps the PR number in the squash title and cuts a long title at a word', () => { - assert.equal(squashTitle('fix(web): a thing', 7), 'fix(web): a thing (#7)'); - const title = squashTitle(LONG_TITLE, 1127); - assert.equal(title, `${LONG_TITLE} (#1127)`); - const longer = squashTitle(`${LONG_TITLE} and more words here`, 1127); - assert.ok(longer.length <= MAX_LINE); - assert.equal(longer, `${LONG_TITLE} (#1127)`); +test('splits a long word between characters, never inside an emoji', () => { + const wrapped = wrapLine(`a${'😀'.repeat(50)}`); + assert.ok(wrapped.every((line) => line.length <= MAX_LINE)); + assert.ok(wrapped.every((line) => !/[\uD800-\uDBFF]$/.test(line))); + assert.equal(wrapped.join(''), `a${'😀'.repeat(50)}`); + assert.throws(() => wrapLine('x y', 0), RangeError); +}); + +test('keeps the PR number in the squash title, or refuses a title that cannot carry it', () => { + assert.equal( + squashTitle(' fix(web): a thing ', 7), + 'fix(web): a thing (#7)' + ); + assert.equal(squashTitle(LONG_TITLE, 1127), `${LONG_TITLE} (#1127)`); + // Cutting these would leave an empty subject or a start-case one. + assert.equal(squashTitle(`${LONG_TITLE} and more`, 1127), null); + assert.equal(squashTitle(`fix: ${'a'.repeat(89)}`, 1133), null); + // Not conventional: commitlint ignores `v1.2.3` alone, not with `(#n)`. + assert.equal(squashTitle('v1.2.3', 1133), null); + assert.equal(squashTitle('Update stuff', 1133), null); + assert.equal( + squashTitle('feat(web)!: drop old route', 3), + 'feat(web)!: drop old route (#3)' + ); }); test('gathers the trailers once each into a footer after the commits', () => { @@ -363,6 +380,12 @@ test('gathers the trailers once each into a footer after the commits', () => { assert.equal(body.split(CO_AUTHOR).length, 2); assert.ok(body.includes('\n\n* fix(shared-data): over-ear WH')); assert.equal(squashBody(['fix: a\r\n\r\nbody\r\n']), '* fix: a\n\nbody'); + // A trailer is never wrapped: one too long for commitlint is left out. + const long = `Co-authored-by: ${'a'.repeat(90)} <a@b.com>`; + assert.equal( + squashBody([`fix: a\n\n${long}\n${CO_AUTHOR}`]), + `* fix: a\n\n${CO_AUTHOR}` + ); }); test('the squash message of PR bodies with lines over 100 characters passes commitlint', () => { From 390846f6fc8a7b9842d1ddbde27c15e92e9a8bc9 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:03:14 +0700 Subject: [PATCH 3/3] docs(release): merge-when-green refuses titles that cannot carry (#n) (#1130) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- Docs/Resources/RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Docs/Resources/RELEASE.md b/Docs/Resources/RELEASE.md index 8fdc80d4..34442a31 100644 --- a/Docs/Resources/RELEASE.md +++ b/Docs/Resources/RELEASE.md @@ -11,7 +11,7 @@ This is the release workflow for `rate.vn`. It follows the same model as DOS/DOS ## Normal release -1. Feature PRs go into `dev` as usual: review, green required checks (`Typecheck & Build Monorepo`, `E2E (Playwright)`, `Secret scan (gitleaks)`, `task-claim`), squash merge. `node scripts/ops/merge-when-green.mjs <pr>` writes the squash commit's message itself: the PR title with `(#<pr>)`, then each commit wrapped at 100 characters, with the co-author trailers in a footer. The promotion PR's `commitlint` check lints the head of `dev`, which is usually that squash commit, so it must pass `commitlint.config.ts` (DOS/Rate#1130). +1. Feature PRs go into `dev` as usual: review, green required checks (`Typecheck & Build Monorepo`, `E2E (Playwright)`, `Secret scan (gitleaks)`, `task-claim`), squash merge. `node scripts/ops/merge-when-green.mjs <pr>` writes the squash commit's message itself: the PR title with `(#<pr>)`, then each commit wrapped at 100 characters, with the co-author trailers in a footer. It refuses a PR whose title is not a conventional header or does not fit `<title> (#<pr>)` in 100 characters: shorten the title. The promotion PR's `commitlint` check lints the head of `dev`, which is usually that squash commit, so it must pass `commitlint.config.ts` (DOS/Rate#1130). 2. Check `dev` on its Vercel Preview deployment. 3. Open the promotion PR from `dev` itself, not from a copy branch: `gh pr create --repo DOS/Rate --base main --head dev --title "chore(release): promote dev to main"`