From 1e4066b3c9182511e02f9d709ea7ab753f819c7b Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Thu, 20 Aug 2026 05:30:08 +0000 Subject: [PATCH] ci: fail the build when a commit will be dropped from the changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit release-please parses every commit with `@conventional-commits/parser`, and when the parse throws it catches, writes two `logger.debug` lines and moves on. Nothing turns red: CI is green, the release PR renders normally, and the commit is simply absent. dcb3bacf — `test(components): build path expectations with pathe, not node:path (#427)` — went missing from the 2.12.0 notes that way; the range holds seven `test:` commits and the notes list six. Fidelity is measured, not assumed. Over all 3240 commits on `main`, this oracle and release-please's own `parseConventionalCommits` agree on every one: 67 rejected by both, zero disagreements in either direction. The hazard is live at roughly one commit in a hundred in the current style, and every one of the 29 line-leading call tokens in the whole history sits in the last 400 commits. The trigger is broader than nested parentheses, which is what this commit first claimed. A body line beginning with a call-like token arms the parser's scope rule, which then rejects unless the very next `(`, `)` or line-end is a `)`. So an unclosed call — `writeTemplates(config.root` running to end of line, or a call wrapped across two lines — fails identically. Any `:` or `!` before the paren disarms it, as does one space of indentation, and only the first group on the line is scanned. The reliable repairs are indenting or prefixing; reflowing works only where the parentheses balance once joined. More importantly, 64 of the 67 real rejections are on the SUBJECT, not in the body — missing colons, `Revert "…"`, a space between type and scope. The first version of the error text sent all of them hunting for nested parens in a body that does not have them, while the parser's own message pointed at line 1. The guard now branches on the reported position and lists the shapes for each. Merge commits are exempt. Their subject is `Merge pull request …`, which the parser rejects and release-please is right to drop — failing there would redden `main` for correct behaviour. Thirteen exist in this history, the last from 2026-05-07. The parser is an ordinary devDependency. `pnpm add`-ing it downgraded TypeScript 6.0.3 to 5.9.3, which this commit first blamed on the package; the cause is that `typescript` is a non-optional peer not listed in devDependencies while `@nuxt/module-builder` peers `^5.9.3`, so any root `pnpm add` does it (#451). Appending to `package.json` by hand and running `pnpm install` gives a 33-line pure-addition lockfile diff with `typescript` untouched — so the CI-time install, the only npm fetch in this repository outside `--frozen-lockfile`, is gone. Tests moved from `test/workflows/` to `test/utils/commit-parses.spec.ts`, because `run.sh` executes before `pnpm install` and the parser now lives in `node_modules`. Eighteen accepted shapes are pinned, weighted by what this repository actually writes — `Co-authored-by:` appears in 83% of recent commits, `Claude-Session:` in 74%, a `(#NNN)` subject in 98%, an indented body line in 28% — because a guard that reddened `main` on ordinary messages would be removed within the week. Closes #436. --- .github/scripts/assert-commit-parses.mjs | 104 ++++++++++++++ .github/workflows/ci.yml | 12 ++ .../docs/1.getting-started/4.contribution.md | 1 + package.json | 1 + pnpm-lock.yaml | 33 +++++ test/utils/commit-parses.spec.ts | 127 ++++++++++++++++++ 6 files changed, 278 insertions(+) create mode 100755 .github/scripts/assert-commit-parses.mjs create mode 100644 test/utils/commit-parses.spec.ts diff --git a/.github/scripts/assert-commit-parses.mjs b/.github/scripts/assert-commit-parses.mjs new file mode 100755 index 00000000..b19d5df5 --- /dev/null +++ b/.github/scripts/assert-commit-parses.mjs @@ -0,0 +1,104 @@ +#!/usr/bin/env node +// Fails when a commit message is one release-please will silently drop from the +// changelog. +// +// release-please parses every commit with `@conventional-commits/parser` and, +// when the parse throws, catches it, writes two `logger.debug` lines and moves +// on. Nothing turns red: CI stays green, the release PR renders normally, and +// the commit is simply absent. That is how dcb3bacf — `test(components): build +// path expectations with pathe, not node:path (#427)` — went missing from the +// 2.12.0 notes, which had seven `test:` commits in range and listed six (#436). +// +// Runs on `push` to `main`, not on pull requests. A PR's own commits are often +// the same text that lands, but not always: the squash subject and body can be +// rewritten in the merge dialog, and #440 was. Only the message on `main` is +// certain to be the one release-please reads. Checking HEAD after the merge also +// needs no history, so the default `fetch-depth: 1` suffices. +// +// Fidelity is not assumed. Run over all 3240 commits on `main`, this oracle and +// release-please's own `parseConventionalCommits` agree on every one: 67 +// rejected by both, zero disagreements in either direction. +// +// usage: assert-commit-parses.mjs # reads git HEAD +// assert-commit-parses.mjs --stdin # reads the message on stdin +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { parser } from '@conventional-commits/parser' + +const readingStdin = process.argv.includes('--stdin') + +// A merge commit's subject is `Merge pull request …`, which the parser rejects +// and release-please is right to drop — there is nothing to put in a changelog. +// Failing on it would redden `main` for correct behaviour. This repository +// squash-merges (13 merge commits exist, the last from 2026-05-07), so the case +// is rare rather than impossible. +function isMergeCommit() { + if (readingStdin) return false + const parents = execFileSync('git', ['rev-list', '--parents', '-n1', 'HEAD'], { encoding: 'utf8' }).trim() + return parents.split(/\s+/).length > 2 +} + +function readMessage() { + if (readingStdin) return readFileSync(0, 'utf8').trim() + return execFileSync('git', ['log', '-1', '--format=%B'], { encoding: 'utf8' }).trim() +} + +if (isMergeCommit()) { + console.log('merge commit — release-please drops it by design, nothing to check') + process.exit(0) +} + +const message = readMessage() +const subject = message.split('\n')[0] + +let error = null +try { + parser(message) +} catch (thrown) { + error = thrown instanceof Error ? thrown.message : String(thrown) +} + +if (!error) { + console.log(`commit message parses: ${subject}`) + process.exit(0) +} + +// The parser reports `at LINE:COLUMN`. Line 1 is the subject, and subject +// failures are the overwhelming majority in this repository's history — 64 of +// the 67 rejections — so pointing everyone at the body would send most readers +// hunting for something that is not there. +const onSubject = /\bat 1:\d+/.test(error) + +console.log(`::error::This commit will be dropped from the changelog: ${subject}`) +console.log(`::error::${error}`) +console.log('') + +if (onSubject) { + console.log('The failure is on the SUBJECT line. Shapes that cause it:') + console.log('') + console.log(' Revert "feat(x): …" GitHub\'s revert button — retitle it `revert(x): …`') + console.log(' chore(lint) fix no colon after the scope') + console.log(' fix (Button): … space between type and scope') + console.log(' docs(x) : … space before the colon') + console.log(' chore(process) nothing after the scope') + console.log(' feat!(x): … `!` belongs after the scope: `feat(x)!:`') + console.log(' * fix(x): … a leading list marker') +} else { + console.log('The failure is in the BODY. A line that begins with a call-like token') + console.log('arms the parser\'s scope rule, and it then rejects the line unless the') + console.log('very next `(`, `)` or line-end is a `)`. Both of these fail:') + console.log('') + console.log(' `.toEqual([join(a, b)])` nested parentheses') + console.log(' writeTemplates(config.root unclosed before the line ends') + console.log('') + console.log('The same text mid-line is fine. Reliable repairs, in order of least') + console.log('disruption: indent the line by one space; or put any word, `:` or `!`') + console.log('before the call. Reflowing works only when the parentheses balance on') + console.log('the joined line — it does not help intrinsic nesting or a code fence.') +} + +console.log('') +console.log('Fix it before merging. Once the message is on `main`, correcting it means') +console.log('either an override block on the merged PR body (release-please reads it in') +console.log('place of the message) or rewriting a protected branch.') +process.exit(1) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82f0f011..147a84fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,18 @@ jobs: - name: Prepare run: pnpm run dev:prepare + # release-please drops a commit it cannot parse and says so only at debug + # level, so a missing changelog entry has no red anywhere — dcb3bacf did + # exactly that to the 2.12.0 notes (#436). + # + # `push` only: ci.yml's push trigger is `branches: [main]`, so every push + # event here is the message that actually lands. A PR's commits are usually + # the same text, but the squash subject and body can be rewritten in the + # merge dialog, so only the message on `main` is certain. + - name: Assert the commit message will not be dropped from the changelog + if: github.event_name == 'push' + run: .github/scripts/assert-commit-parses.mjs + - name: Lint run: pnpm run lint diff --git a/docs/content/docs/1.getting-started/4.contribution.md b/docs/content/docs/1.getting-started/4.contribution.md index 08522f29..576b0206 100644 --- a/docs/content/docs/1.getting-started/4.contribution.md +++ b/docs/content/docs/1.getting-started/4.contribution.md @@ -175,6 +175,7 @@ We use [Conventional Commits](https://www.conventionalcommits.org/) for commit m - Use `fix` and `feat` for code changes that affect functionality or logic - Use `docs` for documentation changes and `chore` for maintenance tasks - Use `revert(Scope): …` when undoing a merged change, and write that subject yourself — GitHub's revert button titles the PR `Revert "…"`, which is not a conventional commit and is dropped from the changelog silently +- Don't begin a line of the commit body with a call-like token — `` `.toEqual([join(a, b)])` `` or `writeTemplates(config.root` at the start of a line is rejected by the changelog parser and the whole commit is dropped. The same text mid-line is fine, and indenting the line by one space fixes it ### Making a Pull Request diff --git a/package.json b/package.json index 78a6b494..3fe65d0c 100644 --- a/package.json +++ b/package.json @@ -210,6 +210,7 @@ "vue-component-type-helpers": "^3.3.10" }, "devDependencies": { + "@conventional-commits/parser": "0.4.1", "@nuxt/eslint-config": "^1.17.0", "@nuxt/module-builder": "^1.0.3", "@nuxt/test-utils": "^4.1.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a2f4fe7a..3140d07c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -245,6 +245,9 @@ importers: specifier: ^3.24.0 || ^4.0.0 version: 4.4.3 devDependencies: + '@conventional-commits/parser': + specifier: 0.4.1 + version: 0.4.1 '@nuxt/eslint-config': specifier: ^1.17.0 version: 1.17.0(@typescript-eslint/utils@8.66.0(eslint@10.8.1(jiti@2.7.0)(supports-color@10.2.2))(supports-color@10.2.2)(typescript@6.0.3))(@vue/compiler-sfc@3.5.41)(eslint@10.8.1(jiti@2.7.0)(supports-color@10.2.2))(supports-color@10.2.2)(typescript@6.0.3) @@ -919,6 +922,9 @@ packages: shiki: optional: true + '@conventional-commits/parser@0.4.1': + resolution: {integrity: sha512-H2ZmUVt6q+KBccXfMBhbBF14NlANeqHTXL4qCL6QGbMzrc4HDXyzWuxPxPNbz71f/5UkR5DrycP5VO9u7crahg==} + '@devframes/hub@0.7.14': resolution: {integrity: sha512-Ym3YHkBnpdwhPw7y4YgURZYntQPShb9raRfo60D1Yk4jb1OlnL2GGHw6pt4iLZqHL4vp1P4T6YpBFPPVx4G38A==} peerDependencies: @@ -8343,6 +8349,9 @@ packages: unist-util-find-after@5.0.0: resolution: {integrity: sha512-amQa0Ep2m6hE2g72AugUItjbuM8X8cGQnFoHk0pGfrFeT9GZhzN5SW8nRsiGKK7Aif4CrACPENkA6P/Lw6fHGQ==} + unist-util-is@4.1.0: + resolution: {integrity: sha512-ZOQSsnce92GrxSqlnEEseX0gi7GH9zTJZ0p9dtu87WRb/37mMPO2Ilx1s/t9vBHrFhbgweUwb+t7cIn5dxPhZg==} + unist-util-is@6.0.1: resolution: {integrity: sha512-LsiILbtBETkDz8I9p1dQ0uyRUWuaQzd/cuEeS1hoRSyW5E5XGmTzlwY1OrNzzakGowI9Dr/I8HVaw4hTtnxy8g==} @@ -8352,9 +8361,15 @@ packages: unist-util-stringify-position@4.0.0: resolution: {integrity: sha512-0ASV06AAoKCDkS2+xw5RXJywruurpbC4JZSm7nr7MOt1ojAzvyyaO+UxZf18j8FCF6kmzCZKcAgN/yu2gm2XgQ==} + unist-util-visit-parents@3.1.1: + resolution: {integrity: sha512-1KROIZWo6bcMrZEwiH2UrXDyalAa0uqzWCxCJj6lPOvTve2WkfgCytoDTPaMnodXh1WrXOq0haVYHj99ynJlsg==} + unist-util-visit-parents@6.0.2: resolution: {integrity: sha512-goh1s1TBrqSqukSc8wrjwWhL0hiJxgA8m4kFxGlQ+8FYQ3C/m11FcTs4YYem7V664AhHVvgoQLk890Ssdsr2IQ==} + unist-util-visit@2.0.3: + resolution: {integrity: sha512-iJ4/RczbJMkD0712mGktuGpm/U4By4FfDonL7N/9tATGIF4imikjOuagyMY53tnZq3NP6BcmlrHhEKAfGWjh7Q==} + unist-util-visit@5.1.0: resolution: {integrity: sha512-m+vIdyeCOpdr/QeQCu2EzxX/ohgS8KbnPDgFni4dQsfSCtpz8UqDyY5GjRru8PDKuYn7Fq19j1CQ+nJSsGKOzg==} @@ -9405,6 +9420,11 @@ snapshots: transitivePeerDependencies: - rangi + '@conventional-commits/parser@0.4.1': + dependencies: + unist-util-visit: 2.0.3 + unist-util-visit-parents: 3.1.1 + '@devframes/hub@0.7.14(devframe@0.7.14(@modelcontextprotocol/sdk@1.30.0(supports-color@10.2.2)(zod@4.4.3))(cac@6.7.14)(srvx@0.11.22)(typescript@6.0.3))': dependencies: birpc: 4.0.0 @@ -19618,6 +19638,8 @@ snapshots: '@types/unist': 3.0.3 unist-util-is: 6.0.1 + unist-util-is@4.1.0: {} + unist-util-is@6.0.1: dependencies: '@types/unist': 3.0.3 @@ -19630,11 +19652,22 @@ snapshots: dependencies: '@types/unist': 3.0.3 + unist-util-visit-parents@3.1.1: + dependencies: + '@types/unist': 2.0.11 + unist-util-is: 4.1.0 + unist-util-visit-parents@6.0.2: dependencies: '@types/unist': 3.0.3 unist-util-is: 6.0.1 + unist-util-visit@2.0.3: + dependencies: + '@types/unist': 2.0.11 + unist-util-is: 4.1.0 + unist-util-visit-parents: 3.1.1 + unist-util-visit@5.1.0: dependencies: '@types/unist': 3.0.3 diff --git a/test/utils/commit-parses.spec.ts b/test/utils/commit-parses.spec.ts new file mode 100644 index 00000000..a30e2c7e --- /dev/null +++ b/test/utils/commit-parses.spec.ts @@ -0,0 +1,127 @@ +import { describe, it, expect } from 'vitest' +import { execFileSync } from 'node:child_process' +import { join } from 'node:path' +import { parser } from '@conventional-commits/parser' + +/** + * Guards `.github/scripts/assert-commit-parses.mjs`, which fails CI when a + * commit message is one release-please will silently drop from the changelog + * (#436). + * + * Two properties matter and they pull in opposite directions. The guard has to + * catch what release-please drops — and it has to keep accepting the shapes this + * repository's commits are actually made of, because a guard that reddens `main` + * on ordinary messages would be removed within the week. The accepted list below + * is therefore weighted by real frequency: `Co-authored-by:` appears in 83% of + * recent commits, `Claude-Session:` in 74%, a `(#NNN)` subject in 98%, an + * indented body line in 28%. + * + * Fidelity was established by sweeping all 3240 commits on `main` through both + * this oracle and release-please's own `parseConventionalCommits`: 67 rejected + * by both, zero disagreements either way. + */ +// `process.cwd()` rather than `import.meta.url`: vitest's transform does not +// leave a `file:` URL there. `documented-scripts.spec.ts` resolves the same way. +const SCRIPT = join(process.cwd(), '.github/scripts/assert-commit-parses.mjs') + +function accepts(message: string): boolean { + try { + parser(message) + return true + } catch { + return false + } +} + +/** Runs the real script the way CI does, returning its exit code. */ +function guardExitCode(message: string): number { + try { + execFileSync('node', [SCRIPT, '--stdin'], { input: message, stdio: 'pipe' }) + return 0 + } catch (error) { + return (error as { status?: number }).status ?? -1 + } +} + +describe('commit messages release-please can read', () => { + describe('shapes this repository actually produces', () => { + // Every one of these is common here. If a parser bump ever rejects one, + // the guard would start failing `main` on ordinary work. + it.each([ + ['plain subject', 'fix(Button): resolve hover state'], + ['subject with the squash suffix', 'fix(Button): resolve hover state (#427)'], + ['type without a scope', 'docs: correct the install tab'], + ['breaking marker after the scope', 'feat(Modal)!: drop the fullscreen prop'], + ['slashed scope', 'feat(DropdownMenu/InputMenu/Select): share the item type'], + ['the prescribed revert form', 'revert(Modal): drop the fullscreen prop'], + ['bare revert', 'revert: drop the fullscreen prop'], + ['co-author trailer', 'fix(x): s\n\nBody.\n\nCo-authored-by: Someone '], + ['session-url trailer', 'fix(x): s\n\nBody.\n\nClaude-Session: https://claude.ai/code/session_01ABC'], + ['closes footer', 'fix(x): s\n\nBody.\n\nCloses #427'], + ['breaking-change footer', 'feat(x)!: s\n\nBREAKING CHANGE: the prop is gone.'], + ['port trailer, a line-leading bare token', 'fix(x): s\n\nPort of nuxt/ui@ccd48940.'], + ['indented body line — the repair the guard recommends', 'fix(x): s\n\nBefore:\n `.toEqual([join(a, b)])`'], + ['a call in prose, mid-line', 'fix(x): s\n\nThe spec asserts f(g(a)) in the middle.'], + ['a line opening with a bare paren', 'fix(x): s\n\n(g(a)) opens the line.'], + ['a line-leading call without nesting', 'fix(x): s\n\n.toEqual(outside) is fine.'], + ['a word before the call', 'fix(x): s\n\nsee f(g(a)) here.'], + ['nesting in the subject text', 'fix(x): handle f(g(a)) correctly'] + ])('accepts %s', (_name, message) => { + expect(accepts(message)).toBe(true) + }) + }) + + describe('shapes that vanish from the changelog', () => { + // Body-position failures. Both arm the same rule — a line beginning with a + // call-like token — and differ only in how the parentheses go wrong. + it.each([ + ['line-leading nested call', 'fix(x): s\n\nf(g(a)) opens the line.'], + ['line-leading nested call, backticked', 'fix(x): s\n\n`.toEqual([join(outside, "components")])`.'], + ['line-leading unclosed call', 'fix(x): s\n\nwriteTemplates(config.root || process.cwd'], + ['a call wrapped across two lines', 'fix(x): s\n\nwriteTemplates(\n config.root)'], + // The real one, reduced to the two lines that matter. + ['the dcb3bacf shape', 'test(components): build path expectations with pathe, not node:path (#427)\n\nthe two places fed by `fixtureRoot()`: `.toEqual([outside])` and\n`.toEqual([join(outside, "components")])`.'] + ])('rejects %s', (_name, message) => { + expect(accepts(message)).toBe(false) + }) + + // Subject-position failures are 64 of the 67 rejections in this history — + // the dominant class, and the one the guard's error text has to name first. + it.each([ + ['the GitHub revert button', 'Revert "feat(Modal): add fullscreen prop"'], + ['no colon after the scope', 'chore(lint) fix the config'], + ['space between type and scope', 'fix (ProseA): correct the spacing'], + ['space before the colon', 'docs(typography) : correct the sample'], + ['nothing after the scope', 'chore(process)'], + ['breaking marker before the scope', 'feat!(Modal): drop the prop'], + ['a leading list marker', '* fix(Button): resolve hover state'] + ])('rejects %s', (_name, message) => { + expect(accepts(message)).toBe(false) + }) + }) + + describe('the script itself', () => { + it('exits 0 on a message release-please can read', () => { + expect(guardExitCode('fix(Button): resolve hover state (#427)')).toBe(0) + }) + + it('exits 1 on the message that went missing from 2.12.0', () => { + expect(guardExitCode('test(components): s\n\n`.toEqual([join(outside, "components")])`.')).toBe(1) + }) + + it('names the subject line when that is where the failure is', () => { + // The guard branches on the parser's reported position. Pointing a reader + // at the body when their subject is malformed is the failure mode this + // assertion exists to prevent — it was the first version's behaviour. + let output = '' + try { + execFileSync('node', [SCRIPT, '--stdin'], { input: 'Revert "feat(x): add a prop"', stdio: 'pipe' }) + } catch (error) { + output = String((error as { stdout?: Buffer }).stdout ?? '') + } + + expect(output).toContain('SUBJECT line') + expect(output).not.toContain('The failure is in the BODY') + }) + }) +})