diff --git a/.github/workflows/changeset-policy.yml b/.github/workflows/changeset-policy.yml new file mode 100644 index 000000000..aff9d3233 --- /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@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@820762786026740c76f36085b0efc47a31fe5020 # pinned from v7.0.0 + 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..303643175 --- /dev/null +++ b/scripts/release/check-major-changeset.mjs @@ -0,0 +1,273 @@ +/** + * 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', '\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. + * + * 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 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.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 }; +} + +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 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 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 = [ + { 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 + gateCases.length} 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') : []; +} + +/** + * 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=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(); + 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({ + changedFiles: changedFilesAgainst(baseSha), + labels: parseLabels(process.env['PR_LABELS_JSON']), + readFile: (path) => readFileSync(path, 'utf8'), + readBaseFile: (path) => readBaseFile(baseSha, path), + }); + + 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 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 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); +} + +if (process.argv[1] !== undefined && import.meta.url.endsWith(process.argv[1].replaceAll('\\', '/'))) { + main(); +}