From 8e878d9ee9f06a196ff04ccfb0a4d836c13cb1aa Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 17 Oct 2026 04:20:54 +0000 Subject: [PATCH] fix(ci): exempt dependabot dependency-manifest PRs from Version Gate rule 1 Dependabot never edits VERSION, so hard rule 1 failed every package.json PR it opened. The workflow now passes the PR author and changed-file list; the gate skips rule 1 only for dependabot[bot] when every file is a package.json, bun.lock(b) or .github/dependabot.yml. A failed file lookup keeps the rule. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .github/workflows/version-gate.yml | 4 ++ scripts/compare-pr-version.ts | 14 ++++++ test/compare-pr-version.test.ts | 79 ++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 test/compare-pr-version.test.ts diff --git a/.github/workflows/version-gate.yml b/.github/workflows/version-gate.yml index e034aaa3e..0dff86cb2 100644 --- a/.github/workflows/version-gate.yml +++ b/.github/workflows/version-gate.yml @@ -73,6 +73,10 @@ jobs: - name: Compare PR VERSION to next free slot env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_VERSION: ${{ steps.versions.outputs.pr_version }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} run: | + PR_CHANGED_FILES=$(gh api --paginate "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --jq '.[].filename' || true) + export PR_CHANGED_FILES bun run scripts/compare-pr-version.ts next.json "${{ github.event.pull_request.number }}" diff --git a/scripts/compare-pr-version.ts b/scripts/compare-pr-version.ts index 27f746aaa..c2c813bf3 100644 --- a/scripts/compare-pr-version.ts +++ b/scripts/compare-pr-version.ts @@ -77,10 +77,24 @@ console.log(` Queue (${claimed.length} open PRs claiming versions):`); if (claimedList) console.log(claimedList); console.log("::endgroup::"); +// Dependabot never edits VERSION, so hard rule 1 would fail every dependency +// PR it opens. Exempt it only when the author is dependabot and every changed +// file is a dependency manifest, a lockfile or the dependabot config. The +// workflow passes the author and the file list; an empty list (the lookup +// failed) keeps the rule. +const DEPENDENCY_FILE = /(?:^|\/)(?:package\.json|bun\.lockb?)$|^\.github\/dependabot\.yml$/; +const changedFiles = (process.env.PR_CHANGED_FILES ?? "").split("\n").map((f) => f.trim()).filter(Boolean); +const dependabotManifestOnly = + process.env.PR_AUTHOR === "dependabot[bot]" && changedFiles.length > 0 && changedFiles.every((f) => DEPENDENCY_FILE.test(f)); + // Hard rule 1: this PR's VERSION must be strictly greater than the base // version, otherwise we're not actually bumping. const pBase = parseV((parsed.base_version ?? "").trim()); if (pBase && cmp(pPR, pBase) <= 0) { + if (dependabotManifestOnly) { + console.log(`::notice::${tag} is a dependabot dependency update (${changedFiles.join(", ")}); VERSION bump not required.`); + process.exit(0); + } console.log(`::error::VERSION not bumped: ${tag} claims v${prVersion} but base is v${parsed.base_version}.`); process.exit(1); } diff --git a/test/compare-pr-version.test.ts b/test/compare-pr-version.test.ts new file mode 100644 index 000000000..dd5013ee2 --- /dev/null +++ b/test/compare-pr-version.test.ts @@ -0,0 +1,79 @@ +/** + * Version Gate (scripts/compare-pr-version.ts). Hard rule 1 requires VERSION + * above the base, which dependabot can never satisfy, so a dependabot PR that + * touches only dependency manifests, lockfiles and dependabot config is exempt. + * Every other author or file keeps the rule. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'compare-pr-version.ts'); + +function gate(env: { PR_VERSION: string; PR_AUTHOR?: string; PR_CHANGED_FILES?: string }) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'version-gate-')); + try { + const next = path.join(dir, 'next.json'); + fs.writeFileSync(next, JSON.stringify({ version: '1.91.117.0', base_version: '1.91.116.0', claimed: [{ pr: 3236, branch: 'fix-wave-5', version: '1.91.117.0' }] })); + const run = Bun.spawnSync([process.execPath, 'run', SCRIPT, next, '3227'], { + env: { PATH: process.env.PATH ?? '', ...env }, + timeout: 30_000, + stdout: 'pipe', + stderr: 'pipe', + }); + return { code: run.exitCode, out: run.stdout.toString() + run.stderr.toString() }; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +const DEPENDABOT = 'dependabot[bot]'; + +describe('version gate: dependabot dependency-manifest exemption', () => { + test('an unbumped VERSION still fails for a human author', () => { + const result = gate({ PR_VERSION: '1.91.116.0', PR_AUTHOR: 'garrytan', PR_CHANGED_FILES: 'package.json\nbun.lock' }); + expect(result.code).toBe(1); + expect(result.out).toContain('::error::VERSION not bumped'); + }); + + test('a dependabot PR behind main that touches only manifests, lockfiles and dependabot config passes', () => { + const files = ['package.json', 'bun.lock', 'lib/diagram-render/package.json', 'lib/diagram-render/bun.lock', '.github/dependabot.yml'].join('\n'); + const result = gate({ PR_VERSION: '1.91.106.0', PR_AUTHOR: DEPENDABOT, PR_CHANGED_FILES: files + '\n' }); + expect(result.code).toBe(0); + expect(result.out).toContain('::notice::PR #3227 is a dependabot dependency update'); + expect(result.out).not.toContain('::error::'); + }); + + test('a dependabot PR with any other file keeps hard rule 1', () => { + for (const other of ['scripts/compare-pr-version.ts', 'VERSION', '.github/workflows/version-gate.yml', 'evil-package.json', 'package.json.bak']) { + const result = gate({ PR_VERSION: '1.91.116.0', PR_AUTHOR: DEPENDABOT, PR_CHANGED_FILES: `package.json\nbun.lock\n${other}` }); + expect(result.code).toBe(1); + expect(result.out).toContain('::error::VERSION not bumped'); + } + }); + + test('a missing file list or author keeps hard rule 1', () => { + expect(gate({ PR_VERSION: '1.91.116.0', PR_AUTHOR: DEPENDABOT, PR_CHANGED_FILES: '' }).code).toBe(1); + expect(gate({ PR_VERSION: '1.91.116.0', PR_AUTHOR: DEPENDABOT }).code).toBe(1); + expect(gate({ PR_VERSION: '1.91.116.0', PR_CHANGED_FILES: 'package.json' }).code).toBe(1); + }); + + test('a bumped dependabot VERSION goes through the collision rule as before', () => { + const result = gate({ PR_VERSION: '1.91.117.0', PR_AUTHOR: DEPENDABOT, PR_CHANGED_FILES: 'package.json' }); + expect(result.code).toBe(1); + expect(result.out).toContain('::error::VERSION collision'); + }); + + test('the workflow passes the PR author and its changed-file list to the gate', () => { + const workflow = Bun.YAML.parse(fs.readFileSync(path.join(ROOT, '.github', 'workflows', 'version-gate.yml'), 'utf8')) as any; + const step = workflow.jobs.check.steps.find((s: any) => s.name === 'Compare PR VERSION to next free slot'); + expect(step.env.PR_AUTHOR).toBe('${{ github.event.pull_request.user.login }}'); + expect(step.env.GH_TOKEN).toBe('${{ secrets.GITHUB_TOKEN }}'); + expect(step.run).toContain('pulls/${{ github.event.pull_request.number }}/files'); + expect(step.run).toContain('--paginate'); + expect(step.run).toContain('export PR_CHANGED_FILES'); + expect(workflow.jobs.check.permissions).toEqual({ contents: 'read', 'pull-requests': 'read' }); + }); +});