diff --git a/.github/scripts/check-changeset-coverage.mjs b/.github/scripts/check-changeset-coverage.mjs index 8d2a208..ceee21e 100644 --- a/.github/scripts/check-changeset-coverage.mjs +++ b/.github/scripts/check-changeset-coverage.mjs @@ -28,6 +28,55 @@ for (const entry of workspaceListing) { } const knownNames = new Set(Object.values(dirToName)); +// Optional per-repo hygiene config (.changeset/hygiene.json, all keys optional): +// { +// "releasePackagePaths": { "packages/rrweb/": "posthog-js" }, +// "transitiveReExports": { "": [""] } +// } +// +// releasePackagePaths replaces workspace ownership for files under a repo-relative +// directory with the package that ships them. Targets must be known workspace +// packages. Trailing slashes are optional; the most specific directory wins. +// This makes bundled sources require only the shipping package's changeset, +// without incorrectly reporting that changeset as extra. +// +// transitiveReExports allows a re-exporter's changeset when its source package +// has both source changes and a changeset (e.g. Gradle api(project(":x"))). +let transitiveReExports = {}; +let releasePackagePaths = []; +const hygieneConfigPath = '.changeset/hygiene.json'; +if (existsSync(hygieneConfigPath)) { + try { + const cfg = JSON.parse(readFileSync(hygieneConfigPath, 'utf8')); + if (cfg.transitiveReExports && typeof cfg.transitiveReExports === 'object') { + transitiveReExports = cfg.transitiveReExports; + } + if ( + cfg.releasePackagePaths && + typeof cfg.releasePackagePaths === 'object' && + !Array.isArray(cfg.releasePackagePaths) + ) { + releasePackagePaths = Object.entries(cfg.releasePackagePaths) + .map(([dir, name]) => [dir.replace(/\/$/, ''), name]) + .filter(([dir, name]) => { + if ( + dir.split('/').some((part) => !part || part === '.' || part === '..') || + !knownNames.has(name) + ) { + process.stderr.write( + `Ignoring invalid releasePackagePaths entry in ${hygieneConfigPath}: ${dir} -> ${name}\n`, + ); + return false; + } + return true; + }) + .sort(([a], [b]) => b.length - a.length); + } + } catch (e) { + process.stderr.write(`Could not parse ${hygieneConfigPath}: ${e.message}\n`); + } +} + // 2. Diff vs base. const mergeBase = sh(`git merge-base origin/${baseRef} HEAD`); const changedFiles = sh(`git diff --name-only ${mergeBase}...HEAD`).split('\n').filter(Boolean); @@ -38,6 +87,13 @@ const affected = new Set(); for (const file of changedFiles) { if (file.startsWith('.changeset/')) continue; if (ignoreSuffixes.some((s) => file.endsWith(s))) continue; + const releasePackage = releasePackagePaths.find( + ([dir]) => file === dir || file.startsWith(dir + '/'), + ); + if (releasePackage) { + affected.add(releasePackage[1]); + continue; + } for (const [dir, name] of Object.entries(dirToName)) { if (file === dir || file.startsWith(dir + '/')) { affected.add(name); @@ -53,34 +109,6 @@ const changesetFiles = sh( .split('\n') .filter((f) => f.endsWith('.md') && !f.endsWith('README.md')); -// 4.5. Optional per-repo hygiene config for transitive re-exports. -// -// Schema (.changeset/hygiene.json, all keys optional): -// { -// "transitiveReExports": { -// "": ["", ""] -// } -// } -// -// When has source changes AND is declared in a changeset on this -// PR, declaring any of its re-exporters is treated as legitimate even if no -// source files in that re-exporter changed. Use for cases the workspace graph -// can't see — e.g. Gradle `api(project(":x"))` re-exports where a downstream -// artifact must be republished to deliver an upstream core change to its own -// consumers. -let transitiveReExports = {}; -const hygieneConfigPath = '.changeset/hygiene.json'; -if (existsSync(hygieneConfigPath)) { - try { - const cfg = JSON.parse(readFileSync(hygieneConfigPath, 'utf8')); - if (cfg.transitiveReExports && typeof cfg.transitiveReExports === 'object') { - transitiveReExports = cfg.transitiveReExports; - } - } catch (e) { - process.stderr.write(`Could not parse ${hygieneConfigPath}: ${e.message}\n`); - } -} - const writeOutput = (body) => { if (!body) { process.stdout.write('body=\n'); diff --git a/.github/scripts/check-changeset-coverage.test.mjs b/.github/scripts/check-changeset-coverage.test.mjs new file mode 100644 index 0000000..9798ed2 --- /dev/null +++ b/.github/scripts/check-changeset-coverage.test.mjs @@ -0,0 +1,186 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import test from 'node:test'; + +const script = fileURLToPath(new URL('./check-changeset-coverage.mjs', import.meta.url)); +const packages = { + 'packages/browser': 'posthog-js', + 'packages/rrweb/rrweb': '@posthog/rrweb', + 'packages/rrweb/types': '@posthog/rrweb-types', + 'packages/rrweb-extra': 'rrweb-extra', + 'packages/node': 'posthog-node', +}; +const mapping = { releasePackagePaths: { 'packages/rrweb/': 'posthog-js' } }; + +function report(t, { files = [], declared = [], config } = {}) { + const cwd = realpathSync(mkdtempSync(join(tmpdir(), 'changeset-hygiene-'))); + t.after(() => rmSync(cwd, { recursive: true, force: true })); + function write(path, content) { + mkdirSync(dirname(join(cwd, path)), { recursive: true }); + writeFileSync(join(cwd, path), content); + } + const git = (...args) => execFileSync('git', args, { cwd, stdio: 'pipe' }); + git('init', '-b', 'main'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + git('config', 'commit.gpgsign', 'false'); + git('config', 'core.hooksPath', '/dev/null'); + // Stub only workspace discovery. Exercise the real script, git diff and frontmatter parser. + write( + 'bin/pnpm', + `#!/bin/sh\nprintf '%s\\n' '${JSON.stringify( + Object.entries(packages).map(([path, name]) => ({ path: join(cwd, path), name })), + )}'\n`, + ); + execFileSync('chmod', ['+x', join(cwd, 'bin/pnpm')]); + if (config) write('.changeset/hygiene.json', JSON.stringify(config)); + git('add', '.'); + git('commit', '-m', 'base'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + for (const file of files) write(file, 'changed\n'); + if (declared.length) { + write( + '.changeset/change.md', + `---\n${declared.map((n) => `'${n}': patch`).join('\n')}\n---\n\nChange\n`, + ); + } + git('add', '.'); + git('commit', '--allow-empty', '-m', 'change'); + return execFileSync(process.execPath, [script], { + cwd, + encoding: 'utf8', + env: { ...process.env, BASE_REF: 'main', PATH: `${cwd}/bin:${process.env.PATH}` }, + }); +} + +test('rrweb-only changes are covered by a browser changeset', (t) => { + assert.equal( + report(t, { + files: ['packages/rrweb/rrweb/src/index.ts', 'packages/rrweb/types/src/index.ts'], + declared: ['posthog-js'], + config: mapping, + }), + 'body=\n', + ); +}); + +test('rrweb changes without a changeset request only the browser package', (t) => { + const body = report(t, { files: ['packages/rrweb/rrweb/src/index.ts'], config: mapping }); + assert.match(body, /`posthog-js` is modified but this PR has no changeset/); + assert.doesNotMatch(body, /@posthog\/rrweb/); +}); + +test('an rrweb changeset does not satisfy the browser release requirement', (t) => { + const body = report(t, { + files: ['packages/rrweb/rrweb/src/index.ts'], + declared: ['@posthog/rrweb'], + config: mapping, + }); + assert.match(body, /"posthog-js": patch/); + assert.match( + body, + /\*\*Declared in a changeset but no source files modified:\*\*\n- `@posthog\/rrweb`/, + ); +}); + +test('unmapped packages still need their own changesets', (t) => { + const body = report(t, { + files: ['packages/rrweb/rrweb/src/index.ts', 'packages/node/src/index.ts'], + declared: ['posthog-js'], + config: mapping, + }); + assert.match(body, /`posthog-node` is modified but not declared/); + assert.doesNotMatch(body, /@posthog\/rrweb/); +}); + +test('path matching respects directory boundaries', (t) => { + const body = report(t, { files: ['packages/rrweb-extra/src/index.ts'], config: mapping }); + assert.match(body, /`rrweb-extra` is modified/); + assert.doesNotMatch(body, /posthog-js/); +}); + +test('the most specific mapping wins, with or without trailing slashes', (t) => { + assert.equal( + report(t, { + files: ['packages/rrweb/rrweb/src/index.ts', 'packages/rrweb/types/src/index.ts'], + declared: ['posthog-js', 'posthog-node'], + config: { + releasePackagePaths: { + 'packages/rrweb': 'posthog-js', + 'packages/rrweb/types/': 'posthog-node', + }, + }, + }), + 'body=\n', + ); +}); + +test('changelog and manifest changes remain ignored under mapped paths', (t) => { + assert.equal( + report(t, { + files: ['packages/rrweb/rrweb/CHANGELOG.md', 'packages/rrweb/types/package.json'], + config: mapping, + }), + 'body=\n', + ); +}); + +test('no config preserves normal workspace coverage', (t) => { + assert.equal( + report(t, { + files: ['packages/rrweb/rrweb/src/index.ts'], + declared: ['@posthog/rrweb'], + }), + 'body=\n', + ); +}); + +test('transitive re-export config still works', (t) => { + assert.equal( + report(t, { + files: ['packages/node/src/index.ts'], + declared: ['posthog-node', 'posthog-js'], + config: { transitiveReExports: { 'posthog-node': ['posthog-js'] } }, + }), + 'body=\n', + ); +}); + +test('browser and bundled source changes share one changeset', (t) => { + assert.equal( + report(t, { + files: ['packages/browser/src/index.ts', 'packages/rrweb/rrweb/src/index.ts'], + declared: ['posthog-js'], + config: mapping, + }), + 'body=\n', + ); +}); + +for (const [path, target] of [ + ['packages/rrweb/', 'typo'], + ['', 'posthog-js'], + ['../packages/rrweb', 'posthog-js'], +]) { + test(`invalid mapping ${path} -> ${target} keeps workspace coverage`, (t) => { + assert.equal( + report(t, { + files: ['packages/rrweb/rrweb/src/index.ts'], + declared: ['@posthog/rrweb'], + config: { releasePackagePaths: { [path]: target } }, + }), + 'body=\n', + ); + }); +} + +test('browser changesets without source changes are still reported as extra', (t) => { + assert.match( + report(t, { declared: ['posthog-js'], config: mapping }), + /Changeset declares `posthog-js` but no source files in that package changed/, + ); +}); diff --git a/.github/workflows/changeset-hygiene-tests.yml b/.github/workflows/changeset-hygiene-tests.yml new file mode 100644 index 0000000..1b51748 --- /dev/null +++ b/.github/workflows/changeset-hygiene-tests.yml @@ -0,0 +1,24 @@ +name: Changeset hygiene tests + +on: + pull_request: + merge_group: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: '22' + + - name: Test changeset coverage + run: node --test .github/scripts/check-changeset-coverage.test.mjs diff --git a/AGENTS.md b/AGENTS.md index ab7d055..4e1cfe6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,13 @@ There is no build step and no app. Changes are config (YAML workflows, semgrep r ## Testing -No general test suite. The one locally runnable thing is the semgrep rule tests: +No general test suite. Run the changeset hygiene script tests with: + +```bash +node --test .github/scripts/check-changeset-coverage.test.mjs +``` + +The semgrep rule tests run with: ```bash semgrep --test .semgrep/