From 113eb4cf3342bd8e38d5ea6668071bcb3d08f81d Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Fri, 2 Oct 2026 21:04:40 +0000 Subject: [PATCH] fix(pr): tighten hand-edited version pinning and surface warnings --- scripts/_workspaces.ts | 8 ++++++++ scripts/resolve-pr-target.ts | 18 +++++++++--------- scripts/update-changelog.ts | 23 +++++++++++++++++++---- test/_workspaces.test.ts | 13 +++++++++++++ test/resolve-pr-target.test.ts | 21 ++++++++------------- test/update-changelog-main.test.ts | 14 ++++++++++++-- test/update-changelog.test.ts | 5 +++++ 7 files changed, 74 insertions(+), 28 deletions(-) diff --git a/scripts/_workspaces.ts b/scripts/_workspaces.ts index a279d60..936674c 100644 --- a/scripts/_workspaces.ts +++ b/scripts/_workspaces.ts @@ -183,6 +183,14 @@ export function isPrerelease (version: string): boolean { return /^\d+\.\d+\.\d+-/.test(version) } +/** Identifier that continues the prerelease line of `version`, or `''` for a stable version. */ +export function prereleaseIdentifier (version: string): string { + const pre = version.match(/^\d+\.\d+\.\d+-([0-9a-zA-Z.-]+)$/)?.[1] + if (!pre) return '' + if (/^\d+$/.test(pre)) return '0' + return pre.replace(/\.\d+$/, '') +} + /** * Resolve the version uppt should act on, the single place both `uppt/pr` * (bump source) and `uppt/release` (tag source) agree on so they can never diff --git a/scripts/resolve-pr-target.ts b/scripts/resolve-pr-target.ts index e776c84..8c244e7 100644 --- a/scripts/resolve-pr-target.ts +++ b/scripts/resolve-pr-target.ts @@ -12,14 +12,7 @@ import process from 'node:process' import { appendFileSync } from 'node:fs' import { runMain } from './_cli.ts' - -/** Identifier that continues the prerelease line of `version`, or `''` for a stable version. */ -export function prereleaseIdentifier (version: string): string { - const pre = version.match(/^\d+\.\d+\.\d+-([0-9a-zA-Z.-]+)$/)?.[1] - if (!pre) return '' - if (/^\d+$/.test(pre)) return '0' - return pre.replace(/\.\d+$/, '') -} +import { prereleaseIdentifier } from './_workspaces.ts' export async function main () { const ref = process.env.GITHUB_REF ?? '' @@ -35,6 +28,13 @@ export async function main () { return } + const prerelease = prereleaseIdentifier(branch.slice('release/v'.length)) + if (prerelease && !/^[a-z0-9][a-z0-9.-]*$/.test(prerelease)) { + console.log(`::notice::danielroe/uppt/pr skipped: ${branch} does not carry a valid uppt prerelease identifier.`) + output({ skip: 'true', base: '', prerelease: '' }) + return + } + const repo = process.env.GITHUB_REPOSITORY! const owner = repo.split('/')[0] const token = process.env.GITHUB_TOKEN @@ -53,7 +53,7 @@ export async function main () { output({ skip: 'true', base: '', prerelease: '' }) return } - output({ skip: 'false', base: pr.base.ref, prerelease: prereleaseIdentifier(branch.slice('release/v'.length)) }) + output({ skip: 'false', base: pr.base.ref, prerelease }) } runMain(import.meta.url, main) diff --git a/scripts/update-changelog.ts b/scripts/update-changelog.ts index bca6231..4e44c4a 100644 --- a/scripts/update-changelog.ts +++ b/scripts/update-changelog.ts @@ -34,7 +34,7 @@ import { resolve } from 'node:path' import { runMain } from './_cli.ts' import { makePkgFormatter } from './pkg-format.ts' -import { buildScopeMap, isPrerelease, parseScopesInput, resolveCurrentVersion, resolveWorkspaces, type Workspace } from './_workspaces.ts' +import { buildScopeMap, isPrerelease, parseScopesInput, prereleaseIdentifier, resolveCurrentVersion, resolveWorkspaces, type Workspace } from './_workspaces.ts' import { buildDependencyGraph, DEPENDENCY_FIELDS, propagateReleases, type BumpLevel } from './_dependency-graph.ts' export interface Commit { @@ -884,7 +884,7 @@ async function syncReleaseBranch ( const others = opts.keepOtherChanges ? [...divergence?.changed ?? []].filter(path => !desired.has(path)) : [] if (others.length) { - console.warn(`Cannot rebuild ${opts.branch} (${drift}) without discarding its changes to ${others.join(', ')}.`) + console.log(`::warning::Cannot rebuild ${opts.branch} (${drift}) without discarding its changes to ${others.join(', ')}.`) return false } if (!process.env.GITHUB_TOKEN) { @@ -1068,8 +1068,11 @@ export function resolvePinnedVersion (opts: { branchVersions: Array currentVersion: string prerelease: boolean + /** Prerelease identifier of this run; a branch named for another identifier is not pinned. */ + identifier?: string }): string | null { const nameVersion = opts.headRef.slice('release/v'.length) + if (opts.identifier !== undefined && prereleaseIdentifier(nameVersion) !== opts.identifier) return null const edited = [...new Set(opts.branchVersions.filter((v): v is string => typeof v === 'string' && v !== nameVersion))] if (edited.length > 1) { console.warn(`Ignoring hand-edited versions on ${opts.headRef}: manifests disagree (${edited.join(', ')}).`) @@ -1110,6 +1113,14 @@ async function findTrackReleasePR ( return pr && { number: pr.number, head: pr.head.ref } } +function sameManifest (a: string | null, b: string): boolean { + try { + return JSON.stringify(JSON.parse(a!)) === JSON.stringify(JSON.parse(b)) + } catch { + return false + } +} + function readVersion (source: string | null): string | undefined { try { const version = (JSON.parse(source!) as { version?: unknown }).version @@ -1263,9 +1274,13 @@ export async function main () { branchVersions, currentVersion, prerelease: Boolean(prerelease), + identifier: prerelease ?? '', }) if (version) pinned = { version, pr: trackPR } } + if (pinned && compareVersions(computedVersion.split('-')[0]!, pinned.version.split('-')[0]!) > 0) { + console.log(`::warning::Unreleased commits call for v${computedVersion}, above the hand-set v${pinned.version} on #${pinned.pr.number}.`) + } const newVersion = pinned?.version ?? computedVersion const bump = pinned ? bumpBetween(currentVersion, newVersion) : determineBump(commits) @@ -1308,10 +1323,10 @@ export async function main () { message: `v${newVersion}`, files: buildBumpFileSet({ monorepo, workspaces, rootPkg, rootPkgSource, currentVersion, newVersion }), keepOtherChanges: true, - isCurrent: branch => readVersion(branch) === newVersion, + isCurrent: (branch, desired) => sameManifest(branch, desired), }) if (!synced) { - console.warn(`Leaving the release PR for ${releaseBranch} unchanged until the branch can be rebuilt.`) + console.log(`::warning::Leaving the release PR for ${releaseBranch} unchanged until the branch can be rebuilt.`) return } } diff --git a/test/_workspaces.test.ts b/test/_workspaces.test.ts index 7841db5..609cf01 100644 --- a/test/_workspaces.test.ts +++ b/test/_workspaces.test.ts @@ -10,6 +10,7 @@ import { lockstepVersionFromWorkspaces, parsePackagesInput, parseScopesInput, + prereleaseIdentifier, resolveCurrentVersion, resolveWorkspaces, } from '../scripts/_workspaces.ts' @@ -419,3 +420,15 @@ describe('buildScopeMap', () => { ]) }) }) + +describe('prereleaseIdentifier', () => { + it.each([ + ['1.3.0', ''], + ['5.0.0-beta.0', 'beta'], + ['5.0.0-rc', 'rc'], + ['5.0.0-alpha.1.2', 'alpha.1'], + ['5.0.0-3', '0'], + ])('%s -> %j', (version, id) => { + expect(prereleaseIdentifier(version)).toBe(id) + }) +}) diff --git a/test/resolve-pr-target.test.ts b/test/resolve-pr-target.test.ts index 7f5fe2f..857b360 100644 --- a/test/resolve-pr-target.test.ts +++ b/test/resolve-pr-target.test.ts @@ -3,19 +3,7 @@ import { tmpdir } from 'node:os' import { resolve } from 'node:path' import process from 'node:process' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { main, prereleaseIdentifier } from '../scripts/resolve-pr-target.ts' - -describe('prereleaseIdentifier', () => { - it.each([ - ['1.3.0', ''], - ['5.0.0-beta.0', 'beta'], - ['5.0.0-rc', 'rc'], - ['5.0.0-alpha.1.2', 'alpha.1'], - ['5.0.0-3', '0'], - ])('%s -> %j', (version, id) => { - expect(prereleaseIdentifier(version)).toBe(id) - }) -}) +import { main } from '../scripts/resolve-pr-target.ts' describe('main', () => { let env: NodeJS.ProcessEnv @@ -77,6 +65,13 @@ describe('main', () => { expect(readFileSync(output, 'utf8')).toBe('skip=true\nbase=\nprerelease=\n') }) + it('skips a pushed release branch with an invalid prerelease identifier', async () => { + process.env.GITHUB_REF = 'refs/heads/release/v1.3.0-Beta.1' + await main() + expect(readFileSync(output, 'utf8')).toBe('skip=true\nbase=\nprerelease=\n') + expect(fetch).not.toHaveBeenCalled() + }) + it('throws when the PR lookup fails', async () => { process.env.GITHUB_REF = 'refs/heads/release/v1.3.0' status = 500 diff --git a/test/update-changelog-main.test.ts b/test/update-changelog-main.test.ts index 51fd736..ed4096e 100644 --- a/test/update-changelog-main.test.ts +++ b/test/update-changelog-main.test.ts @@ -640,7 +640,9 @@ describe('lockstep main with a hand-edited version', () => { git.commits = [{ ...FEAT, subject: 'feat!: break a thing (#7)' }] openReleasePR('release/v2.0.0', '> v2.0.0 is the next major release.\n\n## 👉 Changelog\n\nstuff') releaseBranch('release/v2.0.0', { 'package.json': pkgJson('1.2.4') }) + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) await main() + expect(log).toHaveBeenCalledWith(expect.stringContaining('::warning::Unreleased commits call for v2.0.0, above the hand-set v1.2.4')) expect(prUpdate().title).toBe('v1.2.4') expect(prBody()).toContain('> v1.2.4 is the next patch release.') }) @@ -666,6 +668,13 @@ describe('lockstep main with a hand-edited version', () => { expect(JSON.parse(blobContents()[0]!)).toMatchObject({ version: '1.3.0' }) }) + it('rebuilds a pinned manifest whose other fields are stale', async () => { + openReleasePR('release/v1.3.0') + releaseBranch('release/v1.3.0', { 'package.json': JSON.stringify({ name: 'pkg', version: '1.5.0', private: true }) }) + await main() + expect(JSON.parse(blobContents()[0]!)).toEqual({ name: 'pkg', version: '1.5.0' }) + }) + it('resets an edited version that is not above the current one', async () => { openReleasePR('release/v1.3.0') releaseBranch('release/v1.3.0', { 'package.json': pkgJson('1.0.0') }) @@ -693,9 +702,10 @@ describe('lockstep main with a conflicting release branch', () => { baseChanged: ['package.json'], extra: [{ filename: 'NOTES.md' }], }) - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) await main() - expect(warn).toHaveBeenCalledWith(expect.stringContaining('without discarding its changes to NOTES.md')) + expect(log).toHaveBeenCalledWith(expect.stringContaining('::warning::Cannot rebuild release/v1.3.0')) + expect(log).toHaveBeenCalledWith(expect.stringContaining('::warning::Leaving the release PR')) expect(calls.filter(c => c.method !== 'GET')).toEqual([]) }) }) diff --git a/test/update-changelog.test.ts b/test/update-changelog.test.ts index a2eceb0..7a5a564 100644 --- a/test/update-changelog.test.ts +++ b/test/update-changelog.test.ts @@ -1022,6 +1022,11 @@ describe('resolvePinnedVersion', () => { expect(resolvePinnedVersion({ headRef: 'release/v1.3.0-beta.9', currentVersion: '1.3.0-beta.1', prerelease: true, branchVersions: [version] })).toBeNull() }) + it('ignores a branch named for another prerelease identifier', () => { + expect(resolvePinnedVersion({ headRef: 'release/v1.3.0-alpha.0', currentVersion: '1.2.3', prerelease: true, identifier: 'beta', branchVersions: ['1.3.0-alpha.5'] })).toBeNull() + expect(resolvePinnedVersion({ headRef: 'release/v1.3.0-alpha.0', currentVersion: '1.2.3', prerelease: true, identifier: 'alpha', branchVersions: ['1.3.0-alpha.5'] })).toBe('1.3.0-alpha.5') + }) + it('returns null when nothing was edited', () => { expect(resolvePinnedVersion({ ...base, branchVersions: ['1.3.0', undefined] })).toBeNull() }) -- 2.51.2