From 75c6a319b46c6b9b556182a4844a25a84a7d48d2 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 23 Aug 2026 06:37:31 +0000 Subject: [PATCH 1/4] ci(release): reject commit types that have no changelog section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `changelog-sections` replaces the preset's type list wholesale, so a type absent from ours has no entry. For an ordinary commit that means it is dropped. For a breaking one it means something worse: the breaking note keeps the commit alive, but the section rewrite in `conventional-changelog-conventionalcommits` is guarded by `if (entry)`, so `commit.type` stays the raw string and becomes the group title — and `commitGroupOrder.indexOf(title)` returns -1 for an unknown one, which sorts it above index 0. Reproduced before writing anything, by running the real writer at the version release-please uses over this repository's actual config. A `style(Theme)!` commit renders: ### ⚠ BREAKING CHANGES * **Theme:** the legacy palette is gone. ### style * **Theme:** drop the legacy palette ### Features ... which is #437 exactly: a raw lowercase heading above everything real. `assert-commit-parses.mjs` now checks the type as well as the parse. The list comes from `release-please-config.json` rather than being restated — a guard keeping its own copy of the list is one that will eventually disagree with the file it guards, silently and in the permissive direction, and a test asserts the script reads the config so a later simplification has to fail. Rejecting the type is the second of the two options the issue offered, taken over adding hidden entries per type. Twelve types appear in this history with no entry — `playground` 31 times, `doc` 23, `style` 22 — and the list would never be finished, because it cannot cover `feal(init)`, `ix` or `hore`, which are the same failure arriving as a typo. Wrong case is rejected too: release-please would not match `Fix` either, so accepting it hands back a raw `### Fix` heading. Swept over every commit reachable on `main`: one rejection, `dcb3bacf`, which is the commit this guard was originally written about. No new false positives. `pr-title.yml` runs the same check on the PR title, including on `edited` so a corrected title turns green without an empty push. It is early feedback and not the guarantee, and says so: the squash subject can be rewritten in the merge dialog — #440 was — so the check on `main` stays the thing that is certain. The title reaches the script through the environment rather than the command line, since it is arbitrary text from whoever opened the PR. Refs #437 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc --- .github/scripts/assert-commit-parses.mjs | 54 +++++++++++++++++-- .github/workflows/pr-title.yml | 55 +++++++++++++++++++ test/utils/commit-parses.spec.ts | 67 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/pr-title.yml diff --git a/.github/scripts/assert-commit-parses.mjs b/.github/scripts/assert-commit-parses.mjs index b19d5df5..2bd216a7 100755 --- a/.github/scripts/assert-commit-parses.mjs +++ b/.github/scripts/assert-commit-parses.mjs @@ -1,7 +1,8 @@ #!/usr/bin/env node -// Fails when a commit message is one release-please will silently drop from the -// changelog. +// Fails when a commit message is one release-please will not render correctly +// in the changelog. Two ways that happens, both silent. // +// **The parse throws.** // 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 @@ -19,12 +20,37 @@ // release-please's own `parseConventionalCommits` agree on every one: 67 // rejected by both, zero disagreements in either direction. // +// **The type has no section.** `changelog-sections` in +// `release-please-config.json` replaces the preset's type list wholesale, so a +// type absent from ours has no entry. For an ordinary commit that means it is +// dropped. For a **breaking** one it means something worse: the breaking note +// keeps the commit alive, but the section rewrite in +// `conventional-changelog-conventionalcommits`'s `writer-opts.js` is guarded by +// `if (entry)`, so `commit.type` stays the raw string and becomes the group +// title — and `commitGroupOrder.indexOf(title)` returns -1 for an unknown one, +// which sorts it *above* index 0. Reproduced against the real config: a +// `style(Theme)!` commit renders `### style` above `### Features` (#437). +// +// Rejecting the type at the door rather than adding hidden entries per type is +// the deliberate choice of the two the issue offered. Twelve types appear in +// this history with no entry, most of them scopes misused as types, and the +// list would never be finished — it cannot cover `feal(init)`, `ix` or `hore`, +// which are the same failure arriving as a typo. +// // 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' +// Read from the config rather than restating it: a type added there must not +// need a second edit here, and a guard that disagrees with the thing it guards +// is worse than none. +const CONFIG = new URL('../../release-please-config.json', import.meta.url) +const KNOWN_TYPES = new Set( + JSON.parse(readFileSync(CONFIG, 'utf8')).packages['.']['changelog-sections'].map(entry => entry.type) +) + const readingStdin = process.argv.includes('--stdin') // A merge commit's subject is `Merge pull request …`, which the parser rejects @@ -59,7 +85,29 @@ try { } if (!error) { - console.log(`commit message parses: ${subject}`) + // Case-insensitive on purpose, and the comparison below is not: `Fix(x):` + // is captured as `Fix`, misses the set, and is rejected — which is right, + // because release-please would treat it as an unknown type too. + const type = subject.match(/^([a-z]+)/i)?.[1] + // The parse succeeded, so a type is there; the optional chain is for the + // reader rather than for a case that can happen. + if (type && !KNOWN_TYPES.has(type)) { + console.log(`::error::This commit's type has no changelog section: ${subject}`) + console.log('') + console.log(`\`${type}\` is not in \`changelog-sections\` in release-please-config.json.`) + console.log('An ordinary commit of this type is dropped from the changelog. A breaking') + console.log('one is worse: it survives, but under a raw lowercase heading sorted above') + console.log('every real section, because an unknown group title indexes to -1 (#437).') + console.log('') + console.log(`Configured types: ${[...KNOWN_TYPES].join(', ')}`) + console.log('') + console.log('If this is a typo — `feal`, `ix`, `hore` have all happened — fix the subject.') + console.log('If the type is deliberate, either use one that has a section (`chore` covers') + console.log('most of what `style`, `playground` and `cli` were used for) or add an entry') + console.log('to release-please-config.json in the same change.') + process.exit(1) + } + console.log(`commit message parses, type \`${type}\` has a section: ${subject}`) process.exit(0) } diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml new file mode 100644 index 00000000..28e16e78 --- /dev/null +++ b/.github/workflows/pr-title.yml @@ -0,0 +1,55 @@ +name: PR title 🏷️ + +# Early feedback, not the guarantee. `ci.yml` checks the message on `main` after +# the merge, and that check stays: the squash subject and body can be rewritten +# in the merge dialog — #440 was — so only what lands on `main` is certain to be +# what release-please reads. This job exists because finding out afterwards is +# finding out too late. Correcting a message already on `main` means either an +# override block on the merged PR body or rewriting a protected branch. +# +# `edited` is in the list on purpose: a title fixed after a red run has to turn +# the check green without an empty push. +on: + pull_request: + branches: [ main ] + types: [ opened, edited, reopened, synchronize ] + +permissions: + contents: read + +concurrency: + group: pr-title-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + title: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install pnpm + uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6.0.10 + + - name: Install node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + cache: pnpm + + # `--ignore-scripts`: this job needs `@conventional-commits/parser` on disk + # and nothing else. The postinstall work — nuxt prepare, native builds — + # is the bulk of a normal install here and none of it is reachable from a + # title check. + - name: Install dependencies + run: pnpm install --frozen-lockfile --ignore-scripts + + # Through the environment, not the command line: a title is arbitrary text + # from whoever opened the PR, and interpolating it into a `run:` would let + # a backtick or `$(…)` execute on the runner. + - name: Assert the title is a subject release-please can use + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: printf '%s' "$PR_TITLE" | .github/scripts/assert-commit-parses.mjs --stdin diff --git a/test/utils/commit-parses.spec.ts b/test/utils/commit-parses.spec.ts index a30e2c7e..6da7efea 100644 --- a/test/utils/commit-parses.spec.ts +++ b/test/utils/commit-parses.spec.ts @@ -1,7 +1,9 @@ import { describe, it, expect } from 'vitest' import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' import { join } from 'node:path' import { parser } from '@conventional-commits/parser' +import config from '../../release-please-config.json' /** * Guards `.github/scripts/assert-commit-parses.mjs`, which fails CI when a @@ -24,6 +26,9 @@ import { parser } from '@conventional-commits/parser' // leave a `file:` URL there. `documented-scripts.spec.ts` resolves the same way. const SCRIPT = join(process.cwd(), '.github/scripts/assert-commit-parses.mjs') +/** Derived, not listed: adding a section must not need an edit here either. */ +const configuredTypes = config.packages['.']['changelog-sections'].map(entry => entry.type) + function accepts(message: string): boolean { try { parser(message) @@ -100,6 +105,68 @@ describe('commit messages release-please can read', () => { }) }) + /** + * The second way a commit fails to reach the changelog correctly: it parses, + * but its type has no entry in `changelog-sections`. + * + * Reproduced against the real config before writing any of this, by running + * `conventional-changelog-conventionalcommits@6.1.0`'s writer over a + * `style(Theme)!` commit — the output puts `### style` above `### Features`, + * exactly as #437 predicted, because an unknown group title indexes to -1. + */ + describe('types that have no changelog section', () => { + it('reads the type list from the config rather than restating it', () => { + // A guard that keeps its own copy of the list is a guard that will one day + // disagree with the file it guards, silently and in the permissive + // direction. Asserted here so a later "simplification" has to fail a test. + const script = readFileSync(SCRIPT, 'utf8') + expect(script).toContain('release-please-config.json') + expect(script).toContain('changelog-sections') + }) + + it.each(configuredTypes.map(type => [type]))('accepts the configured type `%s`', (type) => { + expect(guardExitCode(`${type}(Button): resolve hover state`)).toBe(0) + }) + + // The twelve types #437 found in this history with no entry. Every one of + // them is dropped when ordinary and mis-rendered when breaking. + it.each([ + ['playground', 31], ['doc', 23], ['style', 22], ['playgrounds', 12], + ['cli', 6], ['demo', 2], ['core', 1], ['ai', 1] + ])('rejects `%s`, used %i times before anyone noticed', (type) => { + expect(guardExitCode(`${type}(Button): resolve hover state`)).toBe(1) + }) + + // No list of types can cover these, which is the argument for checking the + // type against the config instead of maintaining a denylist. + it.each([['feal(init) Init All'], ['ix: something'], ['hore(x): something'], ['eat(x): something']])( + 'rejects the typo %s', (subject) => { + expect(guardExitCode(subject)).toBe(1) + }) + + it('rejects a breaking commit of an unconfigured type — the #437 case itself', () => { + expect(guardExitCode('style(Theme)!: drop the legacy palette\n\nBREAKING CHANGE: gone.')).toBe(1) + }) + + it('rejects a configured type in the wrong case', () => { + // release-please would not match `Fix` either, so accepting it here would + // hand back a raw `### Fix` heading. + expect(guardExitCode('Fix(Button): resolve hover state')).toBe(1) + }) + + it('says which types are configured, so the fix does not need a file hunt', () => { + let output = '' + try { + execFileSync('node', [SCRIPT, '--stdin'], { input: 'style(x): reindent', stdio: 'pipe' }) + } catch (error) { + output = String((error as { stdout?: Buffer }).stdout ?? '') + } + + expect(output).toContain('has no changelog section') + for (const type of configuredTypes) expect(output).toContain(type) + }) + }) + describe('the script itself', () => { it('exits 0 on a message release-please can read', () => { expect(guardExitCode('fix(Button): resolve hover state (#427)')).toBe(0) From 58858a97c83599be345fb1331395e1c81f4537ae Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 23 Aug 2026 06:55:26 +0000 Subject: [PATCH 2/4] ci(release): require ports to name the upstream commit in the subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only the subject reaches `CHANGELOG.md` — the body is not rendered, breaking notes aside — so it is the one place a reader of the release notes can be handed a way back to what was actually ported. Without it a port is indistinguishable from local work in the only artefact most consumers read. The subject stays ours. Copying upstream's own first line was the request as originally put, and it is not what this does: their `Slider` is this fork's `Range`, and §1 of PORTING.md makes that class of rename mandatory, so their wording would put a component this library does not ship into our changelog. The reference points at the commit; the sentence describes what changed here. fix(Range): forward aria attributes to the thumb (nuxt/ui@d6c3802) The trigger is a **new key in `processed`**, not the ledger being edited. That distinction is the whole design: a reconciliation commit — §6 step 4 requires one whenever a run's last entry has no follower — touches the same file and ports nothing, and demanding a reference there would be wrong. Checked against real history rather than fixtures: #467 (bookkeeping) passes, #470 (local work) passes, #466 and #464 (real ports) are flagged. Two mistakes worth recording, both of the shape this repository keeps hitting. The first draft returned an empty array and tested it for truthiness, so every commit was reported as an unnamed port; running it once was enough. The second was quieter: without `HEAD^` the check returns nothing and passed silently, and `actions/checkout` defaults to depth 1 — the same fail-open as the PyYAML fallback removed in #468. It now warns when it cannot see the previous revision, and ci.yml fetches depth 2. `--stdin` skips it: a bare title cannot say what a commit touches, so the PR-title job checks the parse and the type only. Mutation testing caught that the case asserting this was vacuous — run from the repository root it passed whether the guard respected `--stdin` or not, because HEAD there ports nothing. It now stands on a port revision where the HEAD path does fire. Refs #437 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc --- .github/scripts/assert-commit-parses.mjs | 68 ++++++++++++++++++++ .github/workflows/ci.yml | 5 ++ .sync/PORTING.md | 31 +++++++++- test/utils/commit-parses.spec.ts | 79 +++++++++++++++++++++++- 4 files changed, 179 insertions(+), 4 deletions(-) diff --git a/.github/scripts/assert-commit-parses.mjs b/.github/scripts/assert-commit-parses.mjs index 2bd216a7..bf444c24 100755 --- a/.github/scripts/assert-commit-parses.mjs +++ b/.github/scripts/assert-commit-parses.mjs @@ -64,6 +64,55 @@ function isMergeCommit() { return parents.split(/\s+/).length > 2 } +/** + * The upstream SHAs a port adds to the ledger but does not name in its subject. + * + * Only the subject reaches CHANGELOG.md, so this is the one place a reader of + * the release notes can be given a way back to upstream. The trigger is a new + * key in `processed` rather than the file being touched at all: a bookkeeping + * commit reconciling earlier entries — #467 was one — edits the same file and + * ports nothing, and demanding a reference there would be wrong. + * + * A batched port (§6 4b) adds several keys and can only name one; naming any of + * them is enough, since the ledger carries the rest. + * + * Returns an empty array when the commit is not a port, when there is no + * previous revision to compare against, or when reading either side fails — + * this is a changelog-quality check, not a reason to redden `main` because git + * was unavailable. + */ +function portWithoutUpstreamRef(subject) { + if (readingStdin) return [] + + const at = (revision) => { + try { + const raw = execFileSync('git', ['show', `${revision}:.sync/nuxt-ui.json`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }) + return Object.keys(JSON.parse(raw).processed ?? {}) + } catch { + return null + } + } + + const now = at('HEAD') + if (now === null) return [] + + const before = at('HEAD^') + if (before === null) { + // Depth 1, or the file is new. Say so rather than passing quietly: a check + // that is inert in some checkouts and silent about it reads as a green + // check, which is the failure mode this repository has already been bitten + // by once. ci.yml fetches depth 2 for exactly this comparison. + console.log('::warning::cannot read .sync/nuxt-ui.json at HEAD^ — the upstream-reference check did not run (fetch-depth?)') + return [] + } + + const added = now.filter(sha => !before.includes(sha)) + if (added.length === 0) return [] + // Any prefix long enough to be unambiguous; the house form is seven. + if (added.some(sha => new RegExp(`nuxt/ui@${sha.slice(0, 7)}`, 'i').test(subject))) return [] + return added +} + function readMessage() { if (readingStdin) return readFileSync(0, 'utf8').trim() return execFileSync('git', ['log', '-1', '--format=%B'], { encoding: 'utf8' }).trim() @@ -107,6 +156,25 @@ if (!error) { console.log('to release-please-config.json in the same change.') process.exit(1) } + const missingUpstream = portWithoutUpstreamRef(subject) + if (missingUpstream.length > 0) { + console.log(`::error::This port does not name the upstream commit: ${subject}`) + console.log('') + console.log('It adds these entries to `processed` in .sync/nuxt-ui.json:') + for (const sha of missingUpstream) console.log(` ${sha}`) + console.log('') + console.log('Only the subject line reaches CHANGELOG.md — the body is not rendered —') + console.log('so a port that does not carry the reference there is a changelog entry no') + console.log('reader can trace back to upstream. Append it:') + console.log('') + console.log(` ${subject.replace(/\s*\(#\d+\)\s*$/, '')} (nuxt/ui@${missingUpstream[0].slice(0, 7)})`) + console.log('') + console.log('Our own wording stays ours: upstream names components differently') + console.log('(their `Slider` is this fork\'s `Range`), so the link is the reference and') + console.log('the subject is still about our component. See .sync/PORTING.md §6.') + process.exit(1) + } + console.log(`commit message parses, type \`${type}\` has a section: ${subject}`) process.exit(0) } diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6cbf4762..35f58dfa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,11 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # Two, not the default one: assert-commit-parses.mjs compares + # `.sync/nuxt-ui.json` at HEAD against HEAD^ to tell a port from a + # bookkeeping commit. At depth 1 it warns and skips rather than + # passing quietly, but skipping is not what we want here. + fetch-depth: 2 # Pinning actions to commit SHAs is a state; this is the control that keeps # it. The realistic way the pins get lost is not carelessness — it is a diff --git a/.sync/PORTING.md b/.sync/PORTING.md index 645539f0..f6b2f921 100644 --- a/.sync/PORTING.md +++ b/.sync/PORTING.md @@ -617,6 +617,27 @@ happens — a faithful port can break b24ui where it did not break upstream, because this fork's prop or slot surface is not the same. Decide it, write it down, do not inherit it by accident. +**Name the upstream commit in the subject.** The port's subject ends with +`(nuxt/ui@<7-char sha>)`, before the `(#NNN)` GitHub appends: + + fix(Range): forward aria attributes to the thumb (nuxt/ui@d6c3802) + +Only the subject reaches `CHANGELOG.md` — the body is not rendered, notes +aside — so this is the one place a reader of the release notes can be handed a +way back to what was actually ported. Without it a port is indistinguishable +from local work in the only artefact most consumers ever read. + +The subject stays **ours**. Upstream's own first line is not copied in, because +the names do not survive the port: their `Slider` is this fork's `Range`, and +§1 makes that class of rename mandatory, so their wording would put a component +we do not ship into our changelog. The reference points at the commit; the +sentence describes what changed here. + +`assert-commit-parses.mjs` enforces it, and only for real ports — the trigger is +a new key in `processed`, not the ledger being touched, so a reconciliation +commit like #467 is unaffected. A batched port (4b above) names any one of the +SHAs it added; the ledger carries the rest. + **Check dependency parity, not just the queue.** Every `chore(deps)` port bumps only the packages where this fork already sat on upstream's pre-image — correct, since a package deliberately held back must not be dragged along, and @@ -717,16 +738,19 @@ forward, since every commit between the two would then never be judged. 2. Confirm: jsDoc intact, types not weakened, new props have `renderEach` cases, snapshots updated, no unexpected files changed, any new `v-html`/`innerHTML` justified (§5). -3. Check the breaking marker against §6. If the upstream commit is breaking and +3. Check the subject names the upstream commit — `(nuxt/ui@)` — and that + the SHA is one this PR actually ported. CI checks that a reference is + present; only a reader checks that it points at the right commit. +4. Check the breaking marker against §6. If the upstream commit is breaking and our subject is not, the ledger entry has to say why — a §2 divergence that absorbs it — and if it says nothing, that is the finding. The reverse counts too: a port that changes this fork's prop or slot surface needs the marker even when upstream's commit carried none. Nothing downstream re-checks this; release-please takes the subject at its word. -4. **Merge** (squash). If a commit must be skipped, close the PR and record the +5. **Merge** (squash). If a commit must be skipped, close the PR and record the reason in `.sync/log/.md`, then advance the cursor by hand — closing does not advance it. -5. If a fix corrects a recurring mistake, add a rule here and append a dated +6. If a fix corrects a recurring mistake, add a rule here and append a dated line to the changelog below. ## Changelog of rules @@ -763,3 +787,4 @@ forward, since every commit between the two would then never be judged. - 2026-08-21 — checked #159 and added the §2 **`Modal`/`Slideover`/`Drawer` wrap `emits` with `useBlurOnOpen`** invariant. Upstream is not fixed: unovue/reka-ui#1280 has been open since 2024-08-23 with no assignee or PR, and `reka-ui@2.10.3` still calls `hideOthers`, so nothing was removed. Two defects turned up around the workaround instead. The issue documents `grep -rn 'reka-ui#1280' src/ test/` as the way to find every site to revisit, and it matched nothing — only the composable was annotated, as a URL. And the workaround stranded focus on `` after close for any overlay opened without the built-in trigger slot, because reka-ui's fallback capture of `document.activeElement` skips `` and the blur had just produced it; confirmed by A/B, fixed by restoring focus on close, and now covered at the component and composable level. `test/utils/blur-on-open-workaround.spec.ts` turns the manual re-check into a build signal. Last reviewed: 2026-08-21. - 2026-08-23 — closed #98 (PR #468) by adding the §6 rule **carry upstream's breaking marker into our subject**, and the matching §7 reviewer check. The issue's other two actions were already done and are recorded elsewhere: the version arithmetic table in `releasing.md` maps any `BREAKING CHANGE` or `!` to a major, with no v2-line exception, and release automation landed as release-please. What was missing is the one step automation cannot supply. release-please derives the bump from the squashed subject, so the marker is now load-bearing in a way it was not when the CHANGELOG was hand-written and a human read the diff — a dropped `!` used to be a cosmetic slip and is now a semver violation that every gate passes. The rule is stated in both directions on purpose, because a port is not a copy: a §2 divergence can absorb a break that upstream had, and this fork's own prop surface can break where upstream's did not. Last reviewed: 2026-08-23. - 2026-08-23 — fix of #342 (PR #470): added the §2 rule **tag width caps are relative to the field**. `max-w-[180px]` on the tag label was ours — upstream is plain `truncate` — and it ellipsised tags that had room to spare, because a constant knows nothing about the field's width or what shares its row. Settled by measuring in Chromium rather than by reading the spec, which was the only way to tell three plausible readings apart: in a 562px field a tag wanting 590px renders at 199px under the old cap, 412px under `max-w-[70%]` with both tags still on one row, and 562px uncapped with the second tag pushed to the next line. The same measurement caught what review had flagged and reading had not — `input-tags.ts`'s root is `inline-flex` with no width, so the percentage was circular there: the field grew to 590px, overflowing its 562px parent, and clipped the label anyway. `max-w-full` on that root fixes both and is now part of the invariant. The guard went through three drafts, each corrected by mutation rather than by review: it credited one slot with a neighbour's class, then passed on its own comment (which contains the string `min-w-0`), then missed both the six per-size `tagsItem` overrides at deeper indentation and the `(prev) => [...]` slot form — an arrow function's parameter list closes before its body opens, so balancing brackets returned `(prev: string)` and nothing else. It ends at a comma at depth zero, and a self-check asserts the scanner matched something before reporting no offenders. Last reviewed: 2026-08-23. +- 2026-08-23 — added the §6 rule **name the upstream commit in the subject** and the §7 reviewer check, on the maintainer's request that ported commits be traceable from the changelog. Only the subject reaches `CHANGELOG.md`, so that is the only place the reference can go. Upstream's own first line was the request as originally put and is deliberately not what shipped: their `Slider` is this fork's `Range`, and §1 makes that rename mandatory, so copying their wording would name a component this library does not have — the reference points at the commit and the sentence stays about ours. Enforced by `assert-commit-parses.mjs`, which keys on a new entry appearing in `processed` rather than on the ledger being edited, so the reconciliation commits §6 step 4 requires are unaffected — verified against #467, which passes, and against #466 and #464, which are real ports and are flagged. The same pass extended that guard to reject a type with no `changelog-sections` entry (#437): a breaking commit of an unconfigured type keeps its raw lowercase type as the group title and sorts above every real section, reproduced by running the writer release-please uses. Both checks read `release-please-config.json` and the ledger rather than restating either. Last reviewed: 2026-08-23. diff --git a/test/utils/commit-parses.spec.ts b/test/utils/commit-parses.spec.ts index 6da7efea..842185d8 100644 --- a/test/utils/commit-parses.spec.ts +++ b/test/utils/commit-parses.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest' import { execFileSync } from 'node:child_process' -import { readFileSync } from 'node:fs' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' import { join } from 'node:path' import { parser } from '@conventional-commits/parser' import config from '../../release-please-config.json' @@ -167,6 +168,82 @@ describe('commit messages release-please can read', () => { }) }) + /** + * Ports have to be traceable from the changelog, and the subject is the only + * line that gets there. + * + * The check keys on a new entry appearing in `processed` rather than on + * `.sync/nuxt-ui.json` being edited at all — a reconciliation commit touches + * the same file and ports nothing. That distinction is the whole design, so + * it is exercised against real commits below rather than fixtures. + */ + describe('ports naming the upstream commit', () => { + /** + * Runs the guard at a given revision, in a throwaway worktree. + * + * `title` switches it to the `--stdin` path while keeping the same git + * context, which is what makes the "not on a bare title" case below mean + * anything: run from the repository root it would pass either way, because + * HEAD there ports nothing. + */ + function guardAt(revision: string, title?: string): { code: number, output: string } { + const dir = mkdtempSync(join(tmpdir(), 'b24ui-port-')) + const args = title === undefined ? [SCRIPT] : [SCRIPT, '--stdin'] + try { + execFileSync('git', ['worktree', 'add', '-q', '--detach', dir, revision], { stdio: 'pipe' }) + try { + const output = execFileSync('node', args, { cwd: dir, encoding: 'utf8', stdio: 'pipe', input: title }) + return { code: 0, output } + } catch (error) { + const e = error as { status?: number, stdout?: Buffer } + return { code: e.status ?? -1, output: String(e.stdout ?? '') } + } + } finally { + execFileSync('git', ['worktree', 'remove', '--force', dir], { stdio: 'pipe' }) + rmSync(dir, { recursive: true, force: true }) + } + } + + // #467 reconciled four ledger entries and ported nothing. Requiring an + // upstream reference there would be wrong, and this is the case that makes + // "new key in `processed`" the trigger instead of "file changed". + it('leaves a bookkeeping commit alone', () => { + expect(guardAt('30b4c1fb').code).toBe(0) + }) + + // A real port from before the rule existed. Flagged, which is the point: + // the guard runs forward from here, and this is what it will catch. + it('flags a port whose subject does not name upstream', () => { + const { code, output } = guardAt('4e42a221') + expect(code).toBe(1) + expect(output).toContain('does not name the upstream commit') + // The message has to hand back the exact line to use, or the fix is a + // hunt through the ledger for a SHA. + expect(output).toContain('nuxt/ui@') + }) + + it('leaves ordinary local work alone', () => { + expect(guardAt('1db360ac').code).toBe(0) + }) + + it('is skipped, loudly, when it cannot see the previous revision', () => { + // The check is inert without HEAD^, and silence there would read exactly + // like a pass. ci.yml fetches depth 2 so this warning stays theoretical. + const script = readFileSync(SCRIPT, 'utf8') + expect(script).toContain('::warning::cannot read .sync/nuxt-ui.json at HEAD^') + }) + + it('does not run on a bare title, even standing on a port', () => { + // `--stdin` is the PR-title path: a title alone cannot say what a commit + // touches, so the check must not guess. Asserted at 4e42a221, where the + // HEAD path does fire — from the repository root this would pass whether + // the guard respected `--stdin` or not, and prove nothing. + const title = 'fix(Range): forward aria attributes to the thumb' + expect(guardAt('4e42a221').code, 'the HEAD path must flag this revision').toBe(1) + expect(guardAt('4e42a221', title).code).toBe(0) + }) + }) + describe('the script itself', () => { it('exits 0 on a message release-please can read', () => { expect(guardExitCode('fix(Button): resolve hover state (#427)')).toBe(0) From f8203bd497eb823dc1269d18fd4b7e83bb89fcc1 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 23 Aug 2026 07:44:18 +0000 Subject: [PATCH 3/4] ci(release): take the commit type from the parser, not a second regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found six defects in the two checks this branch adds, three of them undermining the point of it. Every one was confirmed by running rather than reading. **The type check could be walked past.** `/^([a-z]+)/i` takes a letters-only prefix; the parser's type token runs to the first `(`, `!`, `:` or space. So `fix2(x):` read as `fix` and was accepted, `fix-perf(x):` as `fix`, and `2fix(x):` captured nothing at all — skipping the check and printing "type `undefined` has a section", next to a comment of mine calling that a case that could not happen. All three are types release-please would not match, which is the exact bypass this guard exists to close: a typo in letters was caught, a typo with a digit was not. The type now comes from the AST the parse already produced. The grammar has one implementation and it is not this file. Mutation then showed the fix had no test behind it — the original regex still passed all 75 cases, because every unconfigured type in the spec was letters. Four cases added. **The port check flagged commits that port nothing.** It keyed on any new entry in `processed`, but 70 of the ledger's entries are `no-op`, `noop`, `skip` or `n/a`. #442, #439 and #361 were all flagged — correct messages, on `main`, where they can no longer be fixed. The trigger is now a new entry whose `decision` is `port`. **The new tests would have failed every CI run.** They worktree'd onto real SHAs while this same branch sets `fetch-depth: 2`, so the objects are absent on CI; locally they passed only because a dev clone has the history. Rewritten against a synthetic two-commit repository, which also removes a race that bit this branch for real — a concurrent process restoring the same file twice discarded edits mid-review. Three smaller ones. A batch port was required to name every SHA it added, while §6 4b and this file's own docstring say naming one is enough. The ledger key was interpolated into `new RegExp`, so an entry keyed `(a+)+$` hangs the check — it is a substring test against a key checked to be a SHA now. And an unreadable ledger warned at HEAD^ but was silent at HEAD. Two coverage holes closed on review's evidence: disabling `isMergeCommit()` killed no test, and the "reads the type list from the config" test was a string-containment check that a hardcoded copy passes with the comment intact. It now stands the script beside a config naming a type this repository does not configure and requires it to be accepted. Docs corrected where they promised more than the code does. §7 said CI checks the upstream reference; it does, but only on `push` to `main` — the title job works from a title, which cannot say what a commit touched — so the reviewer owns both halves. `pr-title.yml` now says green there does not mean every rule is satisfied. `AGENTS.md` states both requirements, which until now were discoverable only by reddening CI. Nine mutations that previously killed nothing now each kill at least one case. Follow-up #472 covers the install cost of the title job. Refs #437 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc --- .github/scripts/assert-commit-parses.mjs | 104 +++++++---- .github/workflows/pr-title.yml | 6 + .sync/PORTING.md | 7 +- AGENTS.md | 2 +- test/utils/commit-parses.spec.ts | 224 ++++++++++++++++++----- 5 files changed, 259 insertions(+), 84 deletions(-) diff --git a/.github/scripts/assert-commit-parses.mjs b/.github/scripts/assert-commit-parses.mjs index bf444c24..42d7f86e 100755 --- a/.github/scripts/assert-commit-parses.mjs +++ b/.github/scripts/assert-commit-parses.mjs @@ -10,11 +10,15 @@ // 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. +// `ci.yml` runs it on `push` to `main`, which is the check that counts: a PR's +// own commits are often the text that lands, but not always — the squash +// subject and body can be rewritten in the merge dialog, and #440 was. +// `pr-title.yml` also runs it over the PR title with `--stdin`, as early +// feedback rather than as the guarantee. +// +// The parse and type checks work on the message alone. The upstream-reference +// check needs `HEAD^` to see what the commit added to the ledger, so it runs in +// HEAD mode only and `ci.yml` fetches depth 2 for it. // // Fidelity is not assumed. Run over all 3240 commits on `main`, this oracle and // release-please's own `parseConventionalCommits` agree on every one: 67 @@ -69,17 +73,21 @@ function isMergeCommit() { * * Only the subject reaches CHANGELOG.md, so this is the one place a reader of * the release notes can be given a way back to upstream. The trigger is a new - * key in `processed` rather than the file being touched at all: a bookkeeping - * commit reconciling earlier entries — #467 was one — edits the same file and - * ports nothing, and demanding a reference there would be wrong. + * `decision: "port"` entry in `processed`, which is narrower than it looks and has to be. Keying on the file being touched would + * catch the reconciliation commits §6 step 4 requires — #467 is one. Keying on + * any new key would still be wrong: 70 of the ledger's entries are `no-op`, + * `noop`, `skip` or `n/a`, and a commit recording one of those has nothing + * upstream to point a changelog reader at. Review caught that — the first draft + * flagged #442, #439 and #361, all correct as they stand, none fixable once on + * `main`. * - * A batched port (§6 4b) adds several keys and can only name one; naming any of - * them is enough, since the ledger carries the rest. + * A batched port (§6 4b) adds several entries and can only name one; naming any + * of them is enough, since the ledger carries the rest. * - * Returns an empty array when the commit is not a port, when there is no - * previous revision to compare against, or when reading either side fails — - * this is a changelog-quality check, not a reason to redden `main` because git - * was unavailable. + * Returns an empty array when the commit ports nothing or when either side is + * unreadable — this is a changelog-quality check, not a reason to redden `main` + * because git was unavailable. Both unreadable sides warn: a check that is + * inert and silent about it reads exactly like a check that passed. */ function portWithoutUpstreamRef(subject) { if (readingStdin) return [] @@ -87,30 +95,55 @@ function portWithoutUpstreamRef(subject) { const at = (revision) => { try { const raw = execFileSync('git', ['show', `${revision}:.sync/nuxt-ui.json`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }) - return Object.keys(JSON.parse(raw).processed ?? {}) + return JSON.parse(raw).processed ?? {} } catch { return null } } const now = at('HEAD') - if (now === null) return [] + if (now === null) { + console.log('::warning::cannot read .sync/nuxt-ui.json at HEAD — the upstream-reference check did not run') + return [] + } const before = at('HEAD^') if (before === null) { - // Depth 1, or the file is new. Say so rather than passing quietly: a check - // that is inert in some checkouts and silent about it reads as a green - // check, which is the failure mode this repository has already been bitten - // by once. ci.yml fetches depth 2 for exactly this comparison. + // Depth 1, or the file is new. ci.yml fetches depth 2 for this comparison. console.log('::warning::cannot read .sync/nuxt-ui.json at HEAD^ — the upstream-reference check did not run (fetch-depth?)') return [] } - const added = now.filter(sha => !before.includes(sha)) - if (added.length === 0) return [] - // Any prefix long enough to be unambiguous; the house form is seven. - if (added.some(sha => new RegExp(`nuxt/ui@${sha.slice(0, 7)}`, 'i').test(subject))) return [] - return added + const ported = Object.keys(now).filter(sha => !(sha in before) && now[sha]?.decision === 'port') + if (ported.length === 0) return [] + + // A substring test against a key checked to be a SHA, not a pattern. This + // used to build a `new RegExp` from the key, which a ledger entry keyed + // `(a+)+$` turns into a hang — a file is not a place to accept a regular + // expression from. Seven characters is the house form; a longer prefix in + // the subject still contains it, so it matches too. + const unnamed = ported.filter((sha) => { + if (!/^[0-9a-f]{7,40}$/i.test(sha)) { + console.log(`::warning::ledger key is not a SHA, skipping the reference check for it: ${sha}`) + return false + } + return !subject.toLowerCase().includes(`nuxt/ui@${sha.slice(0, 7).toLowerCase()}`) + }) + // Any one is enough. A batch (§6 4b) adds several entries and a subject has + // room for one reference; the ledger carries the rest, and demanding all of + // them would make the rule unfollowable for exactly the case it anticipates. + return unnamed.length === ported.length ? unnamed : [] +} + +/** The `type` token as the parser sees it: everything before `(`, `!` or `:`. */ +function commitType(node) { + if (node === null || typeof node !== 'object') return undefined + if (node.type === 'type' && typeof node.value === 'string') return node.value + for (const child of node.children ?? []) { + const found = commitType(child) + if (found !== undefined) return found + } + return undefined } function readMessage() { @@ -127,20 +160,25 @@ const message = readMessage() const subject = message.split('\n')[0] let error = null +let ast = null try { - parser(message) + ast = parser(message) } catch (thrown) { error = thrown instanceof Error ? thrown.message : String(thrown) } if (!error) { - // Case-insensitive on purpose, and the comparison below is not: `Fix(x):` - // is captured as `Fix`, misses the set, and is rejected — which is right, - // because release-please would treat it as an unknown type too. - const type = subject.match(/^([a-z]+)/i)?.[1] - // The parse succeeded, so a type is there; the optional chain is for the - // reader rather than for a case that can happen. - if (type && !KNOWN_TYPES.has(type)) { + // From the AST the parse above already produced, not re-derived. A second + // regex was the first attempt and it was wrong in both directions: the + // parser's type token runs to the first `(`, `!`, `:` or space, so + // `/^([a-z]+)/i` read `fix2(x):` as `fix` and accepted it, while `2fix(x):` + // captured nothing, skipped the check and printed "type `undefined` has a + // section". Both are types release-please would not match — the exact bypass + // this file exists to close. The grammar has one implementation already. + const type = commitType(ast) + // Case-sensitive: `Fix(x):` misses the set and is rejected, which is right, + // because release-please would not match it either. + if (type !== undefined && !KNOWN_TYPES.has(type)) { console.log(`::error::This commit's type has no changelog section: ${subject}`) console.log('') console.log(`\`${type}\` is not in \`changelog-sections\` in release-please-config.json.`) diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml index 28e16e78..ab1e6b7c 100644 --- a/.github/workflows/pr-title.yml +++ b/.github/workflows/pr-title.yml @@ -9,6 +9,12 @@ name: PR title 🏷️ # # `edited` is in the list on purpose: a title fixed after a red run has to turn # the check green without an empty push. +# +# It checks the parse and the type, and not the `(nuxt/ui@)` reference a +# port has to carry: that one needs to see what the commit changed in the +# ledger, and a title on its own cannot. Green here does not mean every rule in +# assert-commit-parses.mjs is satisfied — PORTING.md §7 puts the reference on +# the reviewer for exactly this reason. on: pull_request: branches: [ main ] diff --git a/.sync/PORTING.md b/.sync/PORTING.md index f6b2f921..778c509b 100644 --- a/.sync/PORTING.md +++ b/.sync/PORTING.md @@ -739,8 +739,11 @@ forward, since every commit between the two would then never be judged. snapshots updated, no unexpected files changed, any new `v-html`/`innerHTML` justified (§5). 3. Check the subject names the upstream commit — `(nuxt/ui@)` — and that - the SHA is one this PR actually ported. CI checks that a reference is - present; only a reader checks that it points at the right commit. + the SHA is one this PR actually ported. **Both halves are yours.** CI does + check that a reference is present, but only on `push` to `main`, after the + squash: the PR-title job works from a title alone, which cannot say what a + commit touches. So a missing reference is something you catch here or find + out about when `main` is already red. 4. Check the breaking marker against §6. If the upstream commit is breaking and our subject is not, the ledger entry has to say why — a §2 divergence that absorbs it — and if it says nothing, that is the finding. The reverse counts diff --git a/AGENTS.md b/AGENTS.md index dd893dea..666a4c57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,7 +65,7 @@ Options: ## Key Conventions -- **Conventional commits**: All commit messages must follow [conventional commits](https://conventionalcommits.org) (e.g. `fix(Button): resolve hover state`, `feat(Modal): add fullscreen prop`). **A revert must be titled `revert(Scope): …`** — GitHub's revert button produces `Revert "…"`, which release-please rejects outright, so the revert reaches no changelog and, if it is the only commit since the last tag, opens no release PR at all. +- **Conventional commits**: All commit messages must follow [conventional commits](https://conventionalcommits.org), and the **type must have a section** in `changelog-sections` (`release-please-config.json`) — one that does not is dropped from the changelog, or, if the commit is breaking, rendered under a raw lowercase heading above every real section. `assert-commit-parses.mjs` rejects it, and rejects a port whose subject does not name the upstream commit as `(nuxt/ui@)` (e.g. `fix(Button): resolve hover state`, `feat(Modal): add fullscreen prop`). **A revert must be titled `revert(Scope): …`** — GitHub's revert button produces `Revert "…"`, which release-please rejects outright, so the revert reaches no changelog and, if it is the only commit since the last tag, opens no release PR at all. - **Releases are automated**: merging work into `main` publishes nothing; release-please keeps one release PR open and merging *that* tags, releases and publishes to npm. Version arithmetic (`feat`/`feature` -> minor, `fix` -> patch), the release cadence commitment, the `severity:crash` hotfix policy, the `revert:` subject a revert PR must carry, and the manual CI approval the release PR needs before it can merge all live in [releasing.md](.github/contributing/releasing.md). Filed here rather than in the References table below, which is scoped to `src/` and `test/` work. - **Semantic colors**: Use `text-description`, `bg-elevated`, etc. — never raw Tailwind palette colors like `text-gray-500`. - **Dependency pins are not ours to move**: `reka-ui` and `vaul-vue` are exact-pinned in `package.json` because upstream pins them at those exact versions; they change through a port, never through a local bump. And `vue` is declared as a required peer (`^3.5.0`) even though upstream does not declare it — `src/` uses `useTemplateRef` and `useId`, both Vue 3.5. Both facts are invariants in [.sync/PORTING.md](.sync/PORTING.md) §2 and guarded by `test/utils/peer-dependencies.spec.ts`. diff --git a/test/utils/commit-parses.spec.ts b/test/utils/commit-parses.spec.ts index 842185d8..b74e5e91 100644 --- a/test/utils/commit-parses.spec.ts +++ b/test/utils/commit-parses.spec.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { execFileSync } from 'node:child_process' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { parser } from '@conventional-commits/parser' @@ -117,12 +117,36 @@ describe('commit messages release-please can read', () => { */ describe('types that have no changelog section', () => { it('reads the type list from the config rather than restating it', () => { - // A guard that keeps its own copy of the list is a guard that will one day - // disagree with the file it guards, silently and in the permissive - // direction. Asserted here so a later "simplification" has to fail a test. - const script = readFileSync(SCRIPT, 'utf8') - expect(script).toContain('release-please-config.json') - expect(script).toContain('changelog-sections') + // Asserted by behaviour, not by grep. The first version checked that the + // script mentioned the config file — which a hardcoded copy of the list + // passes with the comment still in place, as review demonstrated. So: + // stand the script beside a config naming a type this repository does + // not configure, and require it to accept that type. + const dir = mkdtempSync(join(tmpdir(), 'b24ui-config-')) + try { + mkdirSync(join(dir, '.github/scripts'), { recursive: true }) + writeFileSync(join(dir, '.github/scripts/guard.mjs'), readFileSync(SCRIPT, 'utf8')) + writeFileSync(join(dir, 'release-please-config.json'), JSON.stringify({ + packages: { '.': { 'changelog-sections': [{ type: 'invented', section: 'Invented' }] } } + })) + // The copy still imports `@conventional-commits/parser`, resolved by + // walking up from its own directory — so it needs one to walk up to. + symlinkSync(join(process.cwd(), 'node_modules'), join(dir, 'node_modules'), 'dir') + + const run = (subject: string) => { + try { + execFileSync('node', [join(dir, '.github/scripts/guard.mjs'), '--stdin'], { input: subject, stdio: 'pipe' }) + return 0 + } catch (error) { + return (error as { status?: number }).status ?? -1 + } + } + + expect(run('invented(x): a type only that config knows'), 'the config was not read').toBe(0) + expect(run('fix(x): a type only this repository knows'), 'the list is hardcoded').toBe(1) + } finally { + rmSync(dir, { recursive: true, force: true }) + } }) it.each(configuredTypes.map(type => [type]))('accepts the configured type `%s`', (type) => { @@ -145,6 +169,22 @@ describe('commit messages release-please can read', () => { expect(guardExitCode(subject)).toBe(1) }) + // The type token runs to the first `(`, `!`, `:` or space — it is not + // letters. Deriving it with `/^([a-z]+)/i` read `fix2(x):` as `fix` and + // accepted it, and captured nothing at all from `2fix(x):`, printing + // "type `undefined` has a section" and skipping the check. Both are types + // release-please would not match, so both were silent bypasses of the one + // thing this check exists to do. Found by review; the type now comes from + // the parser's own AST, and these cases are what hold it there. + it.each([ + ['a digit inside the type', 'fix2(Button): resolve hover state'], + ['a type starting with a digit', '2fix(Button): resolve hover state'], + ['a hyphen inside the type', 'fix-perf(Button): resolve hover state'], + ['an underscore inside the type', 'fix_button(Button): resolve hover state'] + ])('rejects %s', (_name, subject) => { + expect(guardExitCode(subject)).toBe(1) + }) + it('rejects a breaking commit of an unconfigured type — the #437 case itself', () => { expect(guardExitCode('style(Theme)!: drop the legacy palette\n\nBREAKING CHANGE: gone.')).toBe(1) }) @@ -172,79 +212,167 @@ describe('commit messages release-please can read', () => { * Ports have to be traceable from the changelog, and the subject is the only * line that gets there. * - * The check keys on a new entry appearing in `processed` rather than on - * `.sync/nuxt-ui.json` being edited at all — a reconciliation commit touches - * the same file and ports nothing. That distinction is the whole design, so - * it is exercised against real commits below rather than fixtures. + * Built against a synthetic repository rather than against this one's + * history. The first version worktree'd onto real SHAs — 30b4c1fb, 4e42a221, + * 1db360ac — and review found two things wrong with that. The same PR sets + * ci.yml to `fetch-depth: 2`, so those objects are simply absent on CI and + * all four cases fail every run; and creating worktrees in the repository + * under test races with anything else touching it, which is not theoretical + * — a concurrent run left a mutated script in the working tree while this + * very branch was being reviewed. + * + * A fixture also states the cases plainly. "The commit at 30b4c1fb passes" + * requires the reader to go and find out what that commit is; "a ledger entry + * recording a no-op does not need an upstream reference" does not. */ describe('ports naming the upstream commit', () => { /** - * Runs the guard at a given revision, in a throwaway worktree. - * - * `title` switches it to the `--stdin` path while keeping the same git - * context, which is what makes the "not on a bare title" case below mean - * anything: run from the repository root it would pass either way, because - * HEAD there ports nothing. + * A two-commit repository: a ledger, then a change to it. Returns the + * guard's exit code and output for the second commit. */ - function guardAt(revision: string, title?: string): { code: number, output: string } { - const dir = mkdtempSync(join(tmpdir(), 'b24ui-port-')) - const args = title === undefined ? [SCRIPT] : [SCRIPT, '--stdin'] + function guardOnLedgerChange(before: object, after: object, subject: string) { + const dir = mkdtempSync(join(tmpdir(), 'b24ui-ledger-')) + const git = (...args: string[]) => execFileSync('git', args, { cwd: dir, stdio: 'pipe' }) try { - execFileSync('git', ['worktree', 'add', '-q', '--detach', dir, revision], { stdio: 'pipe' }) + mkdirSync(join(dir, '.sync'), { recursive: true }) + git('init', '-q', '-b', 'main') + git('config', 'user.email', 'spec@example.com') + git('config', 'user.name', 'spec') + + const ledger = join(dir, '.sync/nuxt-ui.json') + writeFileSync(ledger, JSON.stringify({ processed: before }, null, 2)) + git('add', '-A') + git('commit', '-q', '-m', 'chore(sync): the ledger before') + + writeFileSync(ledger, JSON.stringify({ processed: after }, null, 2)) + git('add', '-A') + // `--allow-empty`: the "nothing changed in the ledger" case has no diff + // to commit, and that is exactly the case worth asserting. + git('commit', '-q', '--allow-empty', '-m', subject) + try { - const output = execFileSync('node', args, { cwd: dir, encoding: 'utf8', stdio: 'pipe', input: title }) - return { code: 0, output } + return { code: 0, output: execFileSync('node', [SCRIPT], { cwd: dir, encoding: 'utf8', stdio: 'pipe' }) } } catch (error) { const e = error as { status?: number, stdout?: Buffer } return { code: e.status ?? -1, output: String(e.stdout ?? '') } } } finally { - execFileSync('git', ['worktree', 'remove', '--force', dir], { stdio: 'pipe' }) rmSync(dir, { recursive: true, force: true }) } } - // #467 reconciled four ledger entries and ported nothing. Requiring an - // upstream reference there would be wrong, and this is the case that makes - // "new key in `processed`" the trigger instead of "file changed". - it('leaves a bookkeeping commit alone', () => { - expect(guardAt('30b4c1fb').code).toBe(0) - }) + const PORT = { decision: 'port', pr: 1 } + const SHA_A = 'd6c3802a12ecdc549d605ca8459fc3fbc99af63b' + const SHA_B = 'aa5f4af0b1c2d3e4f5061728394a5b6c7d8e9f01' - // A real port from before the rule existed. Flagged, which is the point: - // the guard runs forward from here, and this is what it will catch. it('flags a port whose subject does not name upstream', () => { - const { code, output } = guardAt('4e42a221') + const { code, output } = guardOnLedgerChange({}, { [SHA_A]: PORT }, 'fix(Range): forward aria attributes') expect(code).toBe(1) expect(output).toContain('does not name the upstream commit') - // The message has to hand back the exact line to use, or the fix is a - // hunt through the ledger for a SHA. - expect(output).toContain('nuxt/ui@') + // The message has to hand back the line to use, or fixing it means a hunt + // through the ledger for a SHA. + expect(output).toContain(`nuxt/ui@${SHA_A.slice(0, 7)}`) + }) + + it('accepts a port that names upstream', () => { + const subject = `fix(Range): forward aria attributes (nuxt/ui@${SHA_A.slice(0, 7)})` + expect(guardOnLedgerChange({}, { [SHA_A]: PORT }, subject).code).toBe(0) }) - it('leaves ordinary local work alone', () => { - expect(guardAt('1db360ac').code).toBe(0) + it('accepts a longer prefix than seven', () => { + const subject = `fix(Range): forward aria attributes (nuxt/ui@${SHA_A.slice(0, 12)})` + expect(guardOnLedgerChange({}, { [SHA_A]: PORT }, subject).code).toBe(0) }) - it('is skipped, loudly, when it cannot see the previous revision', () => { - // The check is inert without HEAD^, and silence there would read exactly - // like a pass. ci.yml fetches depth 2 so this warning stays theoretical. + it('accepts the reference in any case', () => { + const subject = `fix(Range): forward aria attributes (NUXT/UI@${SHA_A.slice(0, 7).toUpperCase()})` + expect(guardOnLedgerChange({}, { [SHA_A]: PORT }, subject).code).toBe(0) + }) + + // §6 4b: a batched port adds several entries and can name only one. + it('accepts a batch that names any one of its ports', () => { + const subject = `fix(Range): port two commits (nuxt/ui@${SHA_B.slice(0, 7)})` + expect(guardOnLedgerChange({}, { [SHA_A]: PORT, [SHA_B]: PORT }, subject).code).toBe(0) + }) + + // 70 of the ledger's entries are not ports. A commit recording one has + // nothing upstream to point a changelog reader at, and requiring a + // reference would redden `main` for a correct message — #442, #439 and + // #361 are real examples the first draft flagged. + it.each([['no-op'], ['noop'], ['skip'], ['n/a']])( + 'leaves a `%s` entry alone', (decision) => { + const entry = { decision, pr: 1 } + expect(guardOnLedgerChange({}, { [SHA_A]: entry }, 'chore(sync): record a skip').code).toBe(0) + }) + + it('leaves a commit that only edits existing entries alone', () => { + // The reconciliation §6 step 4 requires: same keys, new values. + const before = { [SHA_A]: { decision: 'port', b24ui_sha: 'pending-merge' } } + const after = { [SHA_A]: { decision: 'port', b24ui_sha: 'abc1234' } } + expect(guardOnLedgerChange(before, after, 'chore(sync): reconcile the last entry').code).toBe(0) + }) + + it('leaves ordinary work with no ledger change alone', () => { + const same = { [SHA_A]: PORT } + expect(guardOnLedgerChange(same, same, 'fix(Button): resolve hover state').code).toBe(0) + }) + + // A ledger key is data from a file. Building a `RegExp` from it turned an + // entry keyed `(a+)+$` into a hang; it is a substring test against a key + // checked to be a SHA now, and a key that is not one is reported. + it('does not execute a ledger key as a pattern', () => { + const { code, output } = guardOnLedgerChange({}, { '(a+)+$': PORT }, 'fix(x): nuxt/ui@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!X') + expect(code).toBe(0) + expect(output).toContain('not a SHA') + }) + + it('warns rather than passing quietly when it cannot read a side', () => { const script = readFileSync(SCRIPT, 'utf8') + expect(script).toContain('::warning::cannot read .sync/nuxt-ui.json at HEAD —') expect(script).toContain('::warning::cannot read .sync/nuxt-ui.json at HEAD^') }) - it('does not run on a bare title, even standing on a port', () => { + it('does not run on a bare title, even where the HEAD path would fire', () => { // `--stdin` is the PR-title path: a title alone cannot say what a commit - // touches, so the check must not guess. Asserted at 4e42a221, where the - // HEAD path does fire — from the repository root this would pass whether - // the guard respected `--stdin` or not, and prove nothing. - const title = 'fix(Range): forward aria attributes to the thumb' - expect(guardAt('4e42a221').code, 'the HEAD path must flag this revision').toBe(1) - expect(guardAt('4e42a221', title).code).toBe(0) + // touches, so the check must not guess. Paired with the first case above, + // which is the same subject and does fail through the HEAD path. + expect(guardExitCode('fix(Range): forward aria attributes')).toBe(0) }) }) describe('the script itself', () => { + it('skips a merge commit, which release-please drops by design', () => { + // Untested until review's mutation run: disabling `isMergeCommit()` + // entirely, or moving its parent threshold, killed nothing. A merge + // subject is `Merge pull request …`, which the parser rejects and + // release-please is right to drop — failing on it would redden `main` + // for correct behaviour. + const dir = mkdtempSync(join(tmpdir(), 'b24ui-merge-')) + const git = (...args: string[]) => execFileSync('git', args, { cwd: dir, stdio: 'pipe' }) + try { + git('init', '-q', '-b', 'main') + git('config', 'user.email', 'spec@example.com') + git('config', 'user.name', 'spec') + writeFileSync(join(dir, 'f.txt'), 'a') + git('add', '-A') + git('commit', '-q', '-m', 'feat(x): base') + git('checkout', '-q', '-b', 'side') + writeFileSync(join(dir, 'g.txt'), 'b') + git('add', '-A') + git('commit', '-q', '-m', 'feat(x): side') + git('checkout', '-q', 'main') + writeFileSync(join(dir, 'h.txt'), 'c') + git('add', '-A') + git('commit', '-q', '-m', 'feat(x): main') + git('merge', '--no-ff', '-q', '-m', 'Merge pull request #1 from side', 'side') + + const output = execFileSync('node', [SCRIPT], { cwd: dir, encoding: 'utf8', stdio: 'pipe' }) + expect(output).toContain('merge commit') + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + it('exits 0 on a message release-please can read', () => { expect(guardExitCode('fix(Button): resolve hover state (#427)')).toBe(0) }) From 05c3bc83ee67e36a9fa70aed885abeebfef9685f Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Sun, 23 Aug 2026 07:45:46 +0000 Subject: [PATCH 4/4] ci(release): skip the title job when only the PR body changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `edited` fires for a body edit as well, and GitHub offers no title-only trigger. Observed on this PR's own run: updating the description queued a second check for a title nothing had touched. `github.event.changes` names what actually moved and is absent on every other action, so the condition leaves `opened`, `reopened` and `synchronize` alone and only filters the edit case. Measured while it was still unfiltered: the job takes 28 seconds end to end, rather less than the review feared. #472 still stands — it is about not resolving the whole workspace for one parser — but this removes the reruns that had nothing to check. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012MsMuj8Fic9tjWVjyEyrxc --- .github/workflows/pr-title.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml index ab1e6b7c..5a32e912 100644 --- a/.github/workflows/pr-title.yml +++ b/.github/workflows/pr-title.yml @@ -30,6 +30,12 @@ concurrency: jobs: title: runs-on: ubuntu-latest + # `edited` fires for a body edit too, and GitHub does not offer a + # title-only trigger. Observed on this PR's own first run: updating the + # description re-ran the job for a title nothing had touched. `changes` + # names what actually moved, and it is absent on every other action, so + # `opened`, `reopened` and `synchronize` still run. + if: github.event.action != 'edited' || github.event.changes.title != null steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1