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') + }) + }) +})