From 00ed0853b4428edf4316cfa3b957ddd10f531405 Mon Sep 17 00:00:00 2001 From: elkaix Date: Mon, 24 Aug 2026 16:01:48 -0400 Subject: [PATCH 1/3] ci: require maintainer approval for a major changeset A major bump renames the release, breaks every pinned consumer, and cannot be undone once npm has the tarball. The changeset guidance already said it is never a mechanical decision, but nothing enforced it, so a major could ride into main inside a large squash and set the next version on its own. A pull request that adds a major changeset now fails unless it carries the `breaking-change-approved` label. Only added changesets count, so editing prose in a major that already sits on the base branch does not re-gate it. The gate self-tests before it runs: a silent break in the check that decides what ships is worse than a red pull request. --- .github/workflows/changeset-policy.yml | 45 +++++ scripts/release/check-major-changeset.mjs | 193 ++++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 .github/workflows/changeset-policy.yml create mode 100644 scripts/release/check-major-changeset.mjs diff --git a/.github/workflows/changeset-policy.yml b/.github/workflows/changeset-policy.yml new file mode 100644 index 000000000..a212493e8 --- /dev/null +++ b/.github/workflows/changeset-policy.yml @@ -0,0 +1,45 @@ +name: Changeset policy + +# A major bump renames the release and breaks every pinned consumer, and npm +# publishes are irreversible. `gen-changesets` already says a major is never an +# agent's call; this makes that a merge gate rather than a convention. +# +# `labeled`/`unlabeled` are listed so adding the approval label re-runs the +# check instead of leaving a stale red on the pull request. +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + +permissions: + contents: read + +jobs: + major-bump-approval: + name: Major bump needs approval + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - uses: actions/checkout@v4 + with: + # The gate diffs against the base commit, which a shallow clone of + # the merge ref alone does not contain. + fetch-depth: 0 + + - uses: actions/setup-node@v7 + with: + node-version-file: .nvmrc + + # The gate decides what ships, so a silent break in the gate is worse + # than a red pull request. Its own cases run first. + - name: Self-test the gate + run: node scripts/release/check-major-changeset.mjs --self-test + + # Both values reach the script through the environment and are never + # interpolated into a shell command. `base.sha` is used rather than the + # branch name so no caller-chosen text reaches git at all. + - name: Check for an unapproved major changeset + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_LABELS_JSON: ${{ toJSON(github.event.pull_request.labels.*.name) }} + run: node scripts/release/check-major-changeset.mjs diff --git a/scripts/release/check-major-changeset.mjs b/scripts/release/check-major-changeset.mjs new file mode 100644 index 000000000..01b269974 --- /dev/null +++ b/scripts/release/check-major-changeset.mjs @@ -0,0 +1,193 @@ +/** + * Gate: a `major` changeset needs a human decision, recorded on the pull + * request. + * + * `.agents/skills/gen-changesets/SKILL.md` already says never to choose a + * `major` bump alone — stop and get explicit approval. Nothing enforced it, so + * a `major` could ride into `main` inside a large squash and set the next + * release's version on its own. This turns that rule into a check: a pull + * request that ADDS a `major` changeset fails unless it carries the approval + * label. + * + * Only added files count. Editing prose in a `major` changeset that is already + * on the base branch is not a new decision, and re-gating it would block every + * follow-up touching the same file. + */ + +import { readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; + +export const APPROVAL_LABEL = 'breaking-change-approved'; + +/** + * Read the bump levels a changeset declares. + * + * The frontmatter is the block between the first two `---` fences; each entry + * reads `"package": level`. Anything outside that block is the changelog prose + * and must not be scanned — a body that mentions the word "major" is not a + * `major` bump. + * + * @param source - Raw changeset file contents. + * @returns The declared levels, lowercased, in file order. + */ +export function parseBumpLevels(source) { + const normalized = source.replaceAll(/\r\n/gu, '\n'); + if (!normalized.startsWith('---\n')) return []; + const end = normalized.indexOf('\n---', 3); + if (end === -1) return []; + const frontmatter = normalized.slice(4, end + 1); + + const levels = []; + for (const line of frontmatter.split('\n')) { + const match = /^\s*(?:"[^"]+"|'[^']+'|[^:]+)\s*:\s*([A-Za-z]+)\s*$/u.exec(line); + if (match !== null) levels.push(match[1].toLowerCase()); + } + return levels; +} + +/** Changeset paths, ignoring the directory's own README and config. */ +export function isChangesetFile(path) { + return path.startsWith('.changeset/') && path.endsWith('.md') && !path.endsWith('/README.md'); +} + +/** + * Decide whether the gate passes. + * + * @param input.addedFiles - Paths added by the pull request. + * @param input.labels - Label names on the pull request. + * @param input.readFile - Reads one path; injected so this stays pure. + * @returns The offending changesets and whether they are approved. + */ +export function evaluate(input) { + const majors = input.addedFiles + .filter(isChangesetFile) + .filter((path) => parseBumpLevels(input.readFile(path)).includes('major')); + const approved = input.labels.includes(APPROVAL_LABEL); + return { majors, approved, ok: majors.length === 0 || approved }; +} + +function selfTest() { + const cases = [ + { name: 'major in frontmatter', source: '---\n"@pymodel/pythinker-code": major\n---\n\nDrop it.\n', expected: ['major'] }, + { name: 'minor only', source: '---\n"@pymodel/pythinker-code": minor\n---\n\nAdd it.\n', expected: ['minor'] }, + // The body can hold a `key: value` line of its own; only the frontmatter + // declares bumps, so the boundary has to be respected, not just the words. + { name: 'prose with a colon line', source: '---\n"a": patch\n---\n\nBreaking: major\n', expected: ['patch'] }, + { name: 'crlf frontmatter', source: '---\r\n"a": major\r\n---\r\n\r\nText.\r\n', expected: ['major'] }, + { name: 'multi package', source: '---\n"a": patch\n"b": major\n---\n\nText.\n', expected: ['patch', 'major'] }, + { name: 'no frontmatter', source: 'Just prose about a major change.\n', expected: [] }, + { name: 'unterminated frontmatter', source: '---\n"a": major\n', expected: [] }, + ]; + let failures = 0; + for (const { name, source, expected } of cases) { + const actual = parseBumpLevels(source); + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + console.error(`self-test FAILED: ${name} — expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`); + failures += 1; + } + } + + const files = { '.changeset/a.md': '---\n"a": major\n---\n\nText.\n' }; + const readFile = (path) => files[path]; + const blocked = evaluate({ addedFiles: ['.changeset/a.md'], labels: [], readFile }); + if (blocked.ok || blocked.majors.length !== 1) { + console.error('self-test FAILED: an unlabelled major must be blocked'); + failures += 1; + } + const allowed = evaluate({ addedFiles: ['.changeset/a.md'], labels: [APPROVAL_LABEL], readFile }); + if (!allowed.ok) { + console.error('self-test FAILED: a labelled major must pass'); + failures += 1; + } + const readmeOnly = evaluate({ addedFiles: ['.changeset/README.md'], labels: [], readFile: () => '---\n"a": major\n---\n' }); + if (!readmeOnly.ok) { + console.error('self-test FAILED: the changeset README is not a changeset'); + failures += 1; + } + + const labelCases = [ + { name: 'absent', raw: undefined, expected: [] }, + { name: 'empty', raw: '', expected: [] }, + { name: 'json array', raw: '["a","breaking-change-approved"]', expected: ['a', APPROVAL_LABEL] }, + { name: 'not json', raw: 'breaking-change-approved', expected: [] }, + { name: 'not an array', raw: '{"name":"x"}', expected: [] }, + { name: 'non-string members', raw: '[1,"a"]', expected: ['a'] }, + ]; + for (const { name, raw, expected } of labelCases) { + const actual = parseLabels(raw); + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + console.error(`self-test FAILED: labels ${name} — expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`); + failures += 1; + } + } + + if (failures > 0) process.exit(1); + console.log(`check-major-changeset: self-test OK (${cases.length + labelCases.length + 3} cases)`); +} + +/** + * Label names as the workflow passes them: a JSON array, so a label containing + * a comma or a newline cannot smuggle in a second name. + * + * @param raw - The `PR_LABELS_JSON` value, or undefined when unset. + * @returns The label names; empty when the value is absent or not an array. + */ +export function parseLabels(raw) { + if (raw === undefined || raw.length === 0) return []; + let parsed; + try { + parsed = JSON.parse(raw); + } catch { + return []; + } + return Array.isArray(parsed) ? parsed.filter((name) => typeof name === 'string') : []; +} + +function addedFilesAgainst(baseSha) { + const output = execFileSync( + 'git', + ['diff', '--name-only', '--diff-filter=A', `${baseSha}...HEAD`, '--', '.changeset'], + { encoding: 'utf8' }, + ); + return output.split('\n').filter((line) => line.length > 0); +} + +function main() { + if (process.argv.includes('--self-test')) { + selfTest(); + return; + } + + const baseSha = process.env['BASE_SHA']; + if (baseSha === undefined || !/^[0-9a-f]{7,40}$/u.test(baseSha)) { + console.error('check-major-changeset: BASE_SHA must be the pull request base commit.'); + process.exit(1); + } + + const result = evaluate({ + addedFiles: addedFilesAgainst(baseSha), + labels: parseLabels(process.env['PR_LABELS_JSON']), + readFile: (path) => readFileSync(path, 'utf8'), + }); + + if (result.ok) { + const note = result.majors.length === 0 ? 'no new major changeset' : 'major approved by label'; + console.log(`check-major-changeset: OK (${note})`); + return; + } + + console.error('check-major-changeset: FAILED'); + console.error(''); + console.error('This pull request adds a major changeset:'); + for (const path of result.majors) console.error(` - ${path}`); + console.error(''); + console.error('A major bump is a product decision, not a mechanical one: it renames the'); + console.error('release, breaks every pinned consumer, and cannot be walked back once'); + console.error('published. Either lower the bump to minor or patch, or have a maintainer'); + console.error(`add the "${APPROVAL_LABEL}" label to confirm the break is intended.`); + process.exit(1); +} + +if (process.argv[1] !== undefined && import.meta.url.endsWith(process.argv[1].replaceAll('\\', '/'))) { + main(); +} From a67e0e674f7bccbf9d837e87a9c44cf754921fff Mon Sep 17 00:00:00 2001 From: elkaix Date: Mon, 24 Aug 2026 16:42:31 -0400 Subject: [PATCH 2/3] ci: catch a changeset edited up to major, not only a new one The gate listed added changeset files, so a pull request could raise an existing changeset from minor to major and pass unchallenged. List every changeset the pull request adds, edits, or renames into place, and compare each one against the base branch: a file counts only when it declares major now and did not already declare it there. Touching a major that a maintainer already approved no longer asks for the label twice. Pin the workflow's actions to commit SHAs so the gate cannot be changed by repointing a tag. --- .github/workflows/changeset-policy.yml | 4 +- scripts/release/check-major-changeset.mjs | 130 +++++++++++++++++----- 2 files changed, 107 insertions(+), 27 deletions(-) diff --git a/.github/workflows/changeset-policy.yml b/.github/workflows/changeset-policy.yml index a212493e8..aff9d3233 100644 --- a/.github/workflows/changeset-policy.yml +++ b/.github/workflows/changeset-policy.yml @@ -20,13 +20,13 @@ jobs: timeout-minutes: 5 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # pinned from v6.0.2 with: # The gate diffs against the base commit, which a shallow clone of # the merge ref alone does not contain. fetch-depth: 0 - - uses: actions/setup-node@v7 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # pinned from v7.0.0 with: node-version-file: .nvmrc diff --git a/scripts/release/check-major-changeset.mjs b/scripts/release/check-major-changeset.mjs index 01b269974..433199997 100644 --- a/scripts/release/check-major-changeset.mjs +++ b/scripts/release/check-major-changeset.mjs @@ -31,7 +31,7 @@ export const APPROVAL_LABEL = 'breaking-change-approved'; * @returns The declared levels, lowercased, in file order. */ export function parseBumpLevels(source) { - const normalized = source.replaceAll(/\r\n/gu, '\n'); + const normalized = source.replaceAll('\r\n', '\n'); if (!normalized.startsWith('---\n')) return []; const end = normalized.indexOf('\n---', 3); if (end === -1) return []; @@ -53,15 +53,24 @@ export function isChangesetFile(path) { /** * Decide whether the gate passes. * - * @param input.addedFiles - Paths added by the pull request. + * A changeset counts against the pull request when it declares `major` now and + * did not already declare it on the base branch. Editing an existing changeset + * up to `major` therefore counts, while merely touching one that was already + * approved does not ask for the label a second time. + * + * @param input.changedFiles - Changeset paths the pull request adds or edits. * @param input.labels - Label names on the pull request. - * @param input.readFile - Reads one path; injected so this stays pure. + * @param input.readFile - Reads one path at the pull request head. + * @param input.readBaseFile - Reads one path on the base branch, or undefined + * when the path does not exist there. Injected so this stays pure. * @returns The offending changesets and whether they are approved. */ export function evaluate(input) { - const majors = input.addedFiles - .filter(isChangesetFile) - .filter((path) => parseBumpLevels(input.readFile(path)).includes('major')); + const majors = input.changedFiles.filter(isChangesetFile).filter((path) => { + if (!parseBumpLevels(input.readFile(path)).includes('major')) return false; + const base = input.readBaseFile(path); + return base === undefined || !parseBumpLevels(base).includes('major'); + }); const approved = input.labels.includes(APPROVAL_LABEL); return { majors, approved, ok: majors.length === 0 || approved }; } @@ -87,22 +96,76 @@ function selfTest() { } } - const files = { '.changeset/a.md': '---\n"a": major\n---\n\nText.\n' }; + const MAJOR = '---\n"a": major\n---\n\nText.\n'; + const MINOR = '---\n"a": minor\n---\n\nText.\n'; + const files = { '.changeset/a.md': MAJOR }; const readFile = (path) => files[path]; - const blocked = evaluate({ addedFiles: ['.changeset/a.md'], labels: [], readFile }); - if (blocked.ok || blocked.majors.length !== 1) { - console.error('self-test FAILED: an unlabelled major must be blocked'); - failures += 1; - } - const allowed = evaluate({ addedFiles: ['.changeset/a.md'], labels: [APPROVAL_LABEL], readFile }); - if (!allowed.ok) { - console.error('self-test FAILED: a labelled major must pass'); - failures += 1; - } - const readmeOnly = evaluate({ addedFiles: ['.changeset/README.md'], labels: [], readFile: () => '---\n"a": major\n---\n' }); - if (!readmeOnly.ok) { - console.error('self-test FAILED: the changeset README is not a changeset'); - failures += 1; + const absentFromBase = () => undefined; + + const gateCases = [ + { + name: 'an unlabelled new major is blocked', + input: { changedFiles: ['.changeset/a.md'], labels: [], readFile, readBaseFile: absentFromBase }, + ok: false, + majors: 1, + }, + { + name: 'a labelled new major passes', + input: { + changedFiles: ['.changeset/a.md'], + labels: [APPROVAL_LABEL], + readFile, + readBaseFile: absentFromBase, + }, + ok: true, + majors: 1, + }, + { + name: 'the changeset README is not a changeset', + input: { + changedFiles: ['.changeset/README.md'], + labels: [], + readFile: () => MAJOR, + readBaseFile: absentFromBase, + }, + ok: true, + majors: 0, + }, + // The escape this gate exists to close: the file is not new, so a filter on + // added paths alone would never see the bump rise from minor to major. + { + name: 'editing an existing changeset up to major is blocked', + input: { changedFiles: ['.changeset/a.md'], labels: [], readFile, readBaseFile: () => MINOR }, + ok: false, + majors: 1, + }, + { + name: 'touching an already-major changeset does not re-ask for the label', + input: { changedFiles: ['.changeset/a.md'], labels: [], readFile, readBaseFile: () => MAJOR }, + ok: true, + majors: 0, + }, + { + name: 'editing a changeset that stays below major passes', + input: { + changedFiles: ['.changeset/a.md'], + labels: [], + readFile: () => MINOR, + readBaseFile: () => MINOR, + }, + ok: true, + majors: 0, + }, + ]; + + for (const { name, input, ok, majors } of gateCases) { + const actual = evaluate(input); + if (actual.ok !== ok || actual.majors.length !== majors) { + console.error( + `self-test FAILED: ${name} — expected ok=${ok} majors=${majors}, got ok=${actual.ok} majors=${actual.majors.length}`, + ); + failures += 1; + } } const labelCases = [ @@ -122,7 +185,7 @@ function selfTest() { } if (failures > 0) process.exit(1); - console.log(`check-major-changeset: self-test OK (${cases.length + labelCases.length + 3} cases)`); + console.log(`check-major-changeset: self-test OK (${cases.length + labelCases.length + gateCases.length} cases)`); } /** @@ -143,15 +206,31 @@ export function parseLabels(raw) { return Array.isArray(parsed) ? parsed.filter((name) => typeof name === 'string') : []; } -function addedFilesAgainst(baseSha) { +/** + * Changeset paths the pull request adds, edits, or renames into place. + * + * `--diff-filter=d` keeps every status except deletion, so an edit that raises + * an existing changeset to `major` is reported alongside a brand new one. A + * deleted changeset cannot introduce a bump, so it is the only status dropped. + */ +function changedFilesAgainst(baseSha) { const output = execFileSync( 'git', - ['diff', '--name-only', '--diff-filter=A', `${baseSha}...HEAD`, '--', '.changeset'], + ['diff', '--name-only', '--diff-filter=d', `${baseSha}...HEAD`, '--', '.changeset'], { encoding: 'utf8' }, ); return output.split('\n').filter((line) => line.length > 0); } +/** The same path on the base branch, or undefined when it is new there. */ +function readBaseFile(baseSha, path) { + try { + return execFileSync('git', ['show', `${baseSha}:${path}`], { encoding: 'utf8' }); + } catch { + return undefined; + } +} + function main() { if (process.argv.includes('--self-test')) { selfTest(); @@ -165,9 +244,10 @@ function main() { } const result = evaluate({ - addedFiles: addedFilesAgainst(baseSha), + changedFiles: changedFilesAgainst(baseSha), labels: parseLabels(process.env['PR_LABELS_JSON']), readFile: (path) => readFileSync(path, 'utf8'), + readBaseFile: (path) => readBaseFile(baseSha, path), }); if (result.ok) { From f1d8f0eea3643ec812d940a704787ac3dad836b1 Mon Sep 17 00:00:00 2001 From: elkaix Date: Mon, 24 Aug 2026 16:48:51 -0400 Subject: [PATCH 3/3] ci: say who a major break reaches A pinned install keeps using its pinned version; the break lands on consumers who upgrade. Say that, and say "declares" rather than "adds" now that an edited changeset counts too. --- scripts/release/check-major-changeset.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/release/check-major-changeset.mjs b/scripts/release/check-major-changeset.mjs index 433199997..303643175 100644 --- a/scripts/release/check-major-changeset.mjs +++ b/scripts/release/check-major-changeset.mjs @@ -258,13 +258,13 @@ function main() { console.error('check-major-changeset: FAILED'); console.error(''); - console.error('This pull request adds a major changeset:'); + console.error('This pull request declares a major changeset:'); for (const path of result.majors) console.error(` - ${path}`); console.error(''); console.error('A major bump is a product decision, not a mechanical one: it renames the'); - console.error('release, breaks every pinned consumer, and cannot be walked back once'); - console.error('published. Either lower the bump to minor or patch, or have a maintainer'); - console.error(`add the "${APPROVAL_LABEL}" label to confirm the break is intended.`); + console.error('release, breaks every consumer who upgrades to it, and cannot be walked'); + console.error('back once published. Either lower the bump to minor or patch, or have a'); + console.error(`maintainer add the "${APPROVAL_LABEL}" label to confirm the break is intended.`); process.exit(1); }