diff --git a/.github/scripts/assert-commit-parses.mjs b/.github/scripts/assert-commit-parses.mjs index b19d5df5..42d7f86e 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 @@ -9,22 +10,51 @@ // 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 // 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 @@ -38,6 +68,84 @@ 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 + * `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 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 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 [] + + const at = (revision) => { + try { + const raw = execFileSync('git', ['show', `${revision}:.sync/nuxt-ui.json`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }) + return JSON.parse(raw).processed ?? {} + } catch { + return null + } + } + + const now = at('HEAD') + 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. 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 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() { if (readingStdin) return readFileSync(0, 'utf8').trim() return execFileSync('git', ['log', '-1', '--format=%B'], { encoding: 'utf8' }).trim() @@ -52,14 +160,60 @@ 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) { - console.log(`commit message parses: ${subject}`) + // 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.`) + 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) + } + 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/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml new file mode 100644 index 00000000..5a32e912 --- /dev/null +++ b/.github/workflows/pr-title.yml @@ -0,0 +1,67 @@ +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. +# +# 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 ] + 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 + # `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 + 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/.sync/PORTING.md b/.sync/PORTING.md index 645539f0..778c509b 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,22 @@ 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. **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 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 +790,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/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 a30e2c7e..b74e5e91 100644 --- a/test/utils/commit-parses.spec.ts +++ b/test/utils/commit-parses.spec.ts @@ -1,7 +1,10 @@ import { describe, it, expect } from 'vitest' import { execFileSync } from 'node:child_process' +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' +import config from '../../release-please-config.json' /** * Guards `.github/scripts/assert-commit-parses.mjs`, which fails CI when a @@ -24,6 +27,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,7 +106,273 @@ 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', () => { + // 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) => { + 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) + }) + + // 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) + }) + + 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) + }) + }) + + /** + * Ports have to be traceable from the changelog, and the subject is the only + * line that gets there. + * + * 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', () => { + /** + * A two-commit repository: a ledger, then a change to it. Returns the + * guard's exit code and output for the second commit. + */ + 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 { + 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 { + 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 { + rmSync(dir, { recursive: true, force: true }) + } + } + + const PORT = { decision: 'port', pr: 1 } + const SHA_A = 'd6c3802a12ecdc549d605ca8459fc3fbc99af63b' + const SHA_B = 'aa5f4af0b1c2d3e4f5061728394a5b6c7d8e9f01' + + it('flags a port whose subject does not name upstream', () => { + 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 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('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('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 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. 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) })