From 580c002c04eb6c02e1bc898c6b35154c194871da Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Wed, 7 Oct 2026 16:13:05 -0700 Subject: [PATCH 1/2] fix(ci): choose a release's dist-tag when it publishes, and move next after it A stable tag published to latest unconditionally, so a 20.x release tagged after 21.0.0 would move latest back to 20.x. A stable release now goes to latest only when it ranks above npm's latest and every other stable git tag, since a higher release can still be in npm's publish-time malware scan. Otherwise it goes to v-lts, and the job fails if it does not rank above that tag or another release of its major. Every publish now waits until npm lists it, in one npm-publish queue, so the next publish reads the real dist-tags. That stops a queued canary from reading a stale canary tag. After a release reaches latest, next moves up to it, and a failure there prints the npm dist-tag command that finishes it. A re-run of a published release skips npm publish and goes on to the wait and the next move. The steps live in tools/publish-job.js and tools/release-tag.js so specs cover them, and build.sh no longer picks a tag. --- .github/workflows/test.yml | 38 ++++-- .gitignore | 1 - .npmignore | 1 - tools/build.sh | 11 +- tools/publish-job.jasmine.ts | 236 +++++++++++++++++++++++++++++++++++ tools/publish-job.js | 135 ++++++++++++++++++++ tools/release-tag.jasmine.ts | 131 +++++++++++++++++++ tools/release-tag.js | 82 ++++++++++++ 8 files changed, 616 insertions(+), 19 deletions(-) create mode 100644 tools/publish-job.jasmine.ts create mode 100644 tools/publish-job.js create mode 100644 tools/release-tag.jasmine.ts create mode 100644 tools/release-tag.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 666b0d126..0aa4ca3d5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -163,12 +163,22 @@ jobs: name: Publish (NPM) needs: ['build', 'test', 'browser'] if: ${{ github.ref == 'refs/heads/main' || github.event_name == 'release' }} - # One canary publish at a time, so the check below reads the canary the previous one published. + # One npm publish at a time, each held until npm lists it (about 20 minutes at most), so every + # check below reads what the previous publish wrote. + timeout-minutes: 30 concurrency: - group: ${{ github.event_name == 'release' && github.run_id || 'canary-publish' }} + group: npm-publish cancel-in-progress: false queue: max steps: + - name: Checkout the publish scripts + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + sparse-checkout: | + tools/publish-job.js + tools/release-tag.js + sparse-checkout-cone-mode: false - name: Setup node uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: @@ -209,11 +219,25 @@ jobs: exit 1 fi fi + - name: Choose the release's dist-tag + id: release_tag + if: github.event_name == 'release' + run: node tools/publish-job.js release-tag - name: Publish - if: steps.canary_check.outputs.skip != 'true' - run: | - cd ./dist/packages-dist - chmod +x publish.sh - ./publish.sh + id: publish + if: steps.canary_check.outputs.skip != 'true' && steps.release_tag.outputs.published != 'true' + run: npm publish ./dist/packages-dist --access public --registry https://wombat-dressing-room.appspot.com --tag "$NPM_TAG" + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_TAG: ${{ steps.release_tag.outputs.tag || 'canary' }} + - name: Wait for npm to list the version + id: wait + if: steps.publish.outcome == 'success' || steps.release_tag.outputs.published == 'true' + run: node tools/publish-job.js wait "$NPM_TAG" + env: + NPM_TAG: ${{ steps.release_tag.outputs.tag || 'canary' }} + - name: Move next up to the release + if: steps.wait.outputs.listed == 'true' && steps.release_tag.outputs.tag == 'latest' + run: node tools/publish-job.js move-next env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/.gitignore b/.gitignore index beb5d455e..0e6c87259 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,6 @@ coverage api-*.json angularfire.tgz unpack.sh -publish.sh .firebase .angular .vscode \ No newline at end of file diff --git a/.npmignore b/.npmignore index 02a57a315..f1e38dce4 100644 --- a/.npmignore +++ b/.npmignore @@ -1,6 +1,5 @@ *.spec.* test-config.* -publish.sh __ivy_ngcc__/ *.min.js *.min.js.map diff --git a/tools/build.sh b/tools/build.sh index 285f7633c..aaa5683b4 100755 --- a/tools/build.sh +++ b/tools/build.sh @@ -1,13 +1,7 @@ TAG_TEST="^refs/tags/.+$" -LATEST_TEST="^[^-]*$" if [[ $GITHUB_REF =~ $TAG_TEST ]]; then OVERRIDE_VERSION=${GITHUB_REF/refs\/tags\//} - if [[ $OVERRIDE_VERSION =~ $LATEST_TEST ]]; then - NPM_TAG=latest - else - NPM_TAG=next - fi; else PACKAGE_VERSION=$(node -e "console.log(require('./package.json').version)") if ! PUBLISHED_VERSIONS=$(npm view @angular/fire versions --json); then @@ -18,11 +12,8 @@ else # `sha-` stops npm dropping an all-digit sha's leading zero. CANARY_ID=$(TZ=UTC git show -s --date=format-local:%Y%m%d%H%M%S --format=%cd.sha-%h $GITHUB_SHA) OVERRIDE_VERSION=$BASE_VERSION-canary.$CANARY_ID - NPM_TAG=canary fi; npm --no-git-tag-version --allow-same-version -f version $OVERRIDE_VERSION -npm run build && - echo "npm publish . --access public --registry https://wombat-dressing-room.appspot.com --tag $NPM_TAG" > ./dist/packages-dist/publish.sh && - chmod +x ./dist/packages-dist/publish.sh +npm run build diff --git a/tools/publish-job.jasmine.ts b/tools/publish-job.jasmine.ts new file mode 100644 index 000000000..5b3f2286c --- /dev/null +++ b/tools/publish-job.jasmine.ts @@ -0,0 +1,236 @@ +import { execFileSync } from 'child_process'; +import { mkdtempSync, readFileSync, writeFileSync } from 'fs'; +import fsExtra from 'fs-extra'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import * as publishJob from './publish-job.js'; +import 'jasmine'; + +type DistTags = Record; + +interface FakeActionsOptions { + version: string; + // One entry per read of npm's dist-tags. The last entry repeats. + distTagReads: (DistTags | Error)[]; + // Leave out to make any read of the repository's tags fail the spec. + gitTags?: string[]; + npmFails?: boolean; +} + +interface FakeRecord { + logs: string[]; + outputs: Record; + commands: string[]; + sleeps: number; +} + +const unreadable = (url: string) => new Error(`Could not read ${url}: fetch failed (ENOTFOUND).`); +const distTagsUrl = 'https://registry.npmjs.org/-/package/@angular/fire/dist-tags'; + +/** A publish job with no network, no git, no npm and a clock that only `sleep` moves. */ +function fakeActions({ version, distTagReads, gitTags, npmFails = false }: FakeActionsOptions) { + let clock = 0; + const reads = [...distTagReads]; + const record: FakeRecord = { logs: [], outputs: {}, commands: [], sleeps: 0 }; + const actions = { + version: () => version, + fetchDistTags: async () => { + const read = reads.length > 1 ? reads.shift() : reads[0]; + if (read instanceof Error) { + throw read; + } + return read; + }, + gitTags: () => { + if (!gitTags) { + throw new Error('This step must not read the repository\'s tags.'); + } + return gitTags; + }, + run: (command: string, args: string[]) => { + record.commands.push([command, ...args].join(' ')); + if (npmFails) { + throw new Error('npm error code E400'); + } + }, + sleep: async (seconds: number) => { + clock += seconds * 1000; + record.sleeps++; + }, + now: () => clock, + setOutput: (name: string, value: string) => { + record.outputs[name] = value; + }, + log: (message: string) => { + record.logs.push(message); + }, + }; + return { actions, record }; +} + +const tagsAfter21 = ['20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0']; +const moveNextCommand = 'npm dist-tag add @angular/fire@21.0.0 next --registry https://wombat-dressing-room.appspot.com'; + +describe('publish-job.js release-tag', () => { + + it('publishes a new major to latest', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '20.1.0', next: '21.0.0-rc.1' }], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'latest' }); + expect(record.logs).toEqual(['Publishing 21.0.0 under latest.']); + }); + + it('publishes an older major to its lts tag and says what outranks it', async () => { + const { actions, record } = fakeActions({ version: '20.1.1', distTagReads: [{ latest: '21.0.0' }], gitTags: [...tagsAfter21, '20.1.1'] }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'v20-lts' }); + expect(record.logs).toEqual(['::warning::20.1.1 goes to v20-lts, not latest, because it does not rank above 21.0.0.', 'Publishing 20.1.1 under v20-lts.']); + }); + + it('publishes a prerelease to next', async () => { + const { actions, record } = fakeActions({ version: '21.0.0-rc.2', distTagReads: [{ latest: '20.1.0', next: '21.0.0-rc.1' }], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'next' }); + }); + + it('skips publishing on a re-run of a release npm already lists', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }] }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(0); + expect(record.outputs).toEqual({ tag: 'latest', published: 'true' }); + expect(record.logs).toEqual(['npm already lists 21.0.0 as latest, so this run skips publishing it.']); + const lts = fakeActions({ version: '20.1.1', distTagReads: [{ latest: '21.0.0', 'v20-lts': '20.1.1' }] }); + expect(await publishJob.runStep('release-tag', [], lts.actions)).toBe(0); + expect(lts.record.outputs).toEqual({ tag: 'v20-lts', published: 'true' }); + const candidate = fakeActions({ version: '21.1.0-rc.0', distTagReads: [{ latest: '21.0.0', next: '21.1.0-rc.0' }] }); + expect(await publishJob.runStep('release-tag', [], candidate.actions)).toBe(0); + expect(candidate.record.outputs).toEqual({ tag: 'next', published: 'true' }); + }); + + it('fails without choosing a tag when the release must not publish', async () => { + const { actions, record } = fakeActions({ + version: '20.0.4', distTagReads: [{ latest: '21.0.0', 'v20-lts': '20.1.1' }], gitTags: [...tagsAfter21, '20.1.1', '20.0.4'], + }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(1); + expect(record.outputs).toEqual({}); + expect(record.logs).toEqual(['::error::Not publishing 20.0.4 under v20-lts, because it does not rank above 20.1.1, 20.1.0.']); + }); + + it('fails when npm cannot be read', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [unreadable(distTagsUrl)], gitTags: tagsAfter21 }); + expect(await publishJob.runStep('release-tag', [], actions)).toBe(1); + expect(record.logs).toEqual([`::error::Could not read ${distTagsUrl}: fetch failed (ENOTFOUND).`]); + }); +}); + +describe('publish-job.js wait', () => { + const stale = { latest: '20.1.0', canary: '21.0.1-canary.1' }; + const listed = { latest: '21.0.0', canary: '21.0.1-canary.1' }; + + it('waits until npm lists the version under its tag', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [stale, unreadable(distTagsUrl), listed] }); + expect(await publishJob.runStep('wait', ['latest'], actions)).toBe(0); + expect(record.sleeps).toBe(2); + expect(record.outputs).toEqual({ listed: 'true' }); + expect(record.logs).toEqual(['npm lists 21.0.0 as latest, 30s after publishing.']); + }); + + it('fails after 20 minutes for latest, printing the command that moves next', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [stale] }); + expect(await publishJob.runStep('wait', ['latest'], actions)).toBe(1); + expect(record.sleeps).toBe(80); + expect(record.outputs).toEqual({}); + expect(record.logs[0]).toMatch(/^::warning::npm does not list 21\.0\.0 as latest 20 minutes after publishing/); + expect(record.logs[1]).toBe('::error::next was not moved to 21.0.0. Once npm lists 21.0.0, publish rights are required to run: npm dist-tag add @angular/fire@21.0.0 next'); + }); + + it('only warns after 20 minutes for any other tag', async () => { + const { actions, record } = fakeActions({ version: '21.0.1-canary.2', distTagReads: [stale] }); + expect(await publishJob.runStep('wait', ['canary'], actions)).toBe(0); + expect(record.outputs).toEqual({}); + expect(record.logs.length).toBe(1); + expect(record.logs[0]).toMatch(/^::warning::/); + }); +}); + +describe('publish-job.js move-next', () => { + + it('moves next up to the release', async () => { + const { actions, record } = fakeActions({ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }] }); + expect(await publishJob.runStep('move-next', [], actions)).toBe(0); + expect(record.commands).toEqual([moveNextCommand]); + }); + + it('leaves a higher next alone', async () => { + const { actions, record } = fakeActions({ version: '21.0.1', distTagReads: [{ latest: '21.0.1', next: '21.1.0-rc.0' }] }); + expect(await publishJob.runStep('move-next', [], actions)).toBe(0); + expect(record.commands).toEqual([]); + expect(record.logs).toEqual(['Not moving next, because it is 21.1.0-rc.0.']); + }); + + it('prints the command that moves next on every failure', async () => { + const fix = 'Publish rights are required to move it with: npm dist-tag add @angular/fire@21.0.0 next'; + const failures: [FakeActionsOptions, string][] = [ + [{ version: '21.0.0', distTagReads: [unreadable(distTagsUrl)] }, `::error::Could not read ${distTagsUrl}: fetch failed (ENOTFOUND). So next was not checked. ${fix}`], + [{ version: '21.0.0', distTagReads: [{ latest: '21.0.0', next: '21.0.0-rc.1' }], npmFails: true }, `::error::21.0.0 is published, but next was not moved to it. ${fix}`], + ]; + for (const [options, error] of failures) { + const { actions, record } = fakeActions(options); + expect(await publishJob.runStep('move-next', [], actions)).toBe(1); + expect(record.logs).toEqual([error]); + } + }); +}); + + +describe('publish-job.js actions', () => { + const git = (cwd: string, ...args: string[]) => execFileSync('git', ['-c', 'user.name=spec', '-c', 'user.email=spec@example.com', ...args], { cwd, encoding: 'utf8' }).trim(); + const envKeys = ['GITHUB_SERVER_URL', 'GITHUB_REPOSITORY', 'GITHUB_OUTPUT']; + const savedEnv = envKeys.map(key => [key, process.env[key]]); + const savedCwd = process.cwd(); + let workDir: string; + let commits: string[]; + + // A repository named `origin` with three commits on one line, tagged 21.0.0 and 21.0.1, plus an unrelated commit. + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), 'publish-job-')); + const origin = join(workDir, 'origin'); + git(workDir, 'init', '--quiet', 'origin'); + commits = ['one', 'two', 'three'].map(message => { + git(origin, 'commit', '--quiet', '--allow-empty', '-m', message); + return git(origin, 'rev-parse', 'HEAD'); + }); + git(origin, 'tag', '21.0.0', commits[0]); + git(origin, 'tag', '-a', '21.0.1', '-m', 'annotated', commits[1]); + git(origin, 'checkout', '--quiet', '--orphan', 'unrelated'); + git(origin, 'commit', '--quiet', '--allow-empty', '-m', 'unrelated'); + commits.push(git(origin, 'rev-parse', 'HEAD')); + // A file:// address makes git honor --filter, as it does for the real repository. + process.env.GITHUB_SERVER_URL = `file://${workDir}`; + process.env.GITHUB_REPOSITORY = 'origin'; + process.env.GITHUB_OUTPUT = join(workDir, 'output'); + process.chdir(workDir); + }); + + afterEach(() => { + process.chdir(savedCwd); + for (const [key, value] of savedEnv) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + fsExtra.removeSync(workDir); + }); + + it('lists the repository\'s tag names', () => { + expect(publishJob.actions.gitTags().sort()).toEqual(['21.0.0', '21.0.1']); + }); + + it('appends step outputs as name=value lines', () => { + writeFileSync(process.env.GITHUB_OUTPUT ?? '', ''); + publishJob.actions.setOutput('tag', 'latest'); + publishJob.actions.setOutput('listed', 'true'); + expect(readFileSync(process.env.GITHUB_OUTPUT ?? '', 'utf8')).toBe('tag=latest\nlisted=true\n'); + }); +}); diff --git a/tools/publish-job.js b/tools/publish-job.js new file mode 100644 index 000000000..d3101f526 --- /dev/null +++ b/tools/publish-job.js @@ -0,0 +1,135 @@ +// No dependencies: the publish job runs this without installing node_modules. +const { execFileSync } = require('child_process'); +const { appendFileSync, readFileSync } = require('fs'); +const { highestReleaseAbove, movesNext, publishedUnder, releaseTag } = require('./release-tag.js'); + +const DIST_TAGS_URL = 'https://registry.npmjs.org/-/package/@angular/fire/dist-tags'; +const WOMBAT_URL = 'https://wombat-dressing-room.appspot.com'; +// npm lists a version only after its publish-time malware scan. +const WAIT_SECONDS = 1200; +const POLL_SECONDS = 15; + +/** Reads npm's dist-tags from the endpoint npm does not CDN-cache, unlike the package data `npm view` reads. */ +async function fetchDistTags() { + let lastError; + for (const pauseSeconds of [0, 2, 4]) { + await new Promise(resolve => setTimeout(resolve, pauseSeconds * 1000)); + try { + const response = await fetch(DIST_TAGS_URL, { signal: AbortSignal.timeout(30_000) }); + if (!response.ok) { + throw new Error(`it answered ${response.status}`); + } + return await response.json(); + } catch (error) { + lastError = error; + } + } + const reason = lastError.cause ? `${lastError.message} (${lastError.cause.code ?? lastError.cause.message})` : lastError.message; + throw new Error(`Could not read ${DIST_TAGS_URL}: ${reason}.`); +} + +const git = (args, options) => execFileSync('git', args, options); +const repositoryUrl = () => `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}`; + +/** The npm, git, file, clock and log calls the steps make on the GitHub Actions runner. */ +const actions = { + version: () => JSON.parse(readFileSync('dist/packages-dist/package.json', 'utf8')).version, + fetchDistTags, + gitTags: () => git(['ls-remote', '--tags', '--refs', repositoryUrl()], { encoding: 'utf8' }) + .split('\n').filter(line => line).map(line => line.replace(/.*refs\/tags\//, '')), + run: (command, args) => execFileSync(command, args, { stdio: 'inherit' }), + sleep: seconds => new Promise(resolve => setTimeout(resolve, seconds * 1000)), + now: () => Date.now(), + setOutput: (name, value) => appendFileSync(process.env.GITHUB_OUTPUT, `${name}=${value}\n`), + log: message => console.log(message), +}; + +/** + * Chooses the dist-tag a tagged release publishes under and sets the step output `tag`. On a re-run + * of a release npm already lists, also sets `published`, so the job skips `npm publish` and goes on + * to the steps after it. + */ +async function chooseReleaseTag(actions) { + const version = actions.version(); + const distTags = await actions.fetchDistTags(); + const alreadyUnder = publishedUnder(version, distTags); + if (alreadyUnder) { + actions.log(`npm already lists ${version} as ${alreadyUnder}, so this run skips publishing it.`); + actions.setOutput('tag', alreadyUnder); + actions.setOutput('published', 'true'); + return; + } + // A higher release can be tagged and still be in npm's publish-time malware scan, so git tags count too. + const gitTags = actions.gitTags(); + const tag = releaseTag(version, distTags, gitTags); + if (tag.endsWith('-lts')) { + actions.log(`::warning::${version} goes to ${tag}, not latest, because it does not rank above ${highestReleaseAbove(version, distTags, gitTags)}.`); + } + actions.log(`Publishing ${version} under ${tag}.`); + actions.setOutput('tag', tag); +} + +/** + * Waits until npm lists the published version under `tag`, holding the publish queue so the next + * publish reads it, and sets the step output `listed`. Fails on timeout only for `latest`, since + * `next` then cannot be moved. + */ +async function waitUntilListed(actions, tag) { + const version = actions.version(); + const start = actions.now(); + while (actions.now() - start < WAIT_SECONDS * 1000) { + const distTags = await actions.fetchDistTags().catch(() => ({})); + if (distTags[tag] === version) { + actions.log(`npm lists ${version} as ${tag}, ${Math.round((actions.now() - start) / 1000)}s after publishing.`); + actions.setOutput('listed', 'true'); + return; + } + await actions.sleep(POLL_SECONDS); + } + actions.log(`::warning::npm does not list ${version} as ${tag} ${WAIT_SECONDS / 60} minutes after publishing, so the next publish may read the previous ${tag}.`); + if (tag === 'latest') { + throw new Error(`next was not moved to ${version}. Once npm lists ${version}, publish rights are required to run: npm dist-tag add @angular/fire@${version} next`); + } +} + +/** Moves `next` up to a release npm now lists as `latest`, unless `next` is already higher. */ +async function moveNext(actions) { + const version = actions.version(); + const fix = `Publish rights are required to move it with: npm dist-tag add @angular/fire@${version} next`; + + let distTags; + try { distTags = await actions.fetchDistTags(); } + catch (error) { throw new Error(`${error.message} So next was not checked. ${fix}`); } + + if (!movesNext(version, distTags)) { + actions.log(`Not moving next, because it is ${distTags.next}.`); + return; + } + + try { actions.run('npm', ['dist-tag', 'add', `@angular/fire@${version}`, 'next', '--registry', WOMBAT_URL]); } + catch { throw new Error(`${version} is published, but next was not moved to it. ${fix}`); } +} + +const steps = { + 'release-tag': actions => chooseReleaseTag(actions), + 'wait': (actions, tag) => waitUntilListed(actions, tag), + 'move-next': actions => moveNext(actions), +}; + +/** Runs one step, turning a thrown error into an `::error::` line and exit code 1. */ +async function runStep(name, args, actions) { + try { + await steps[name](actions, ...args); + return 0; + } catch (error) { + actions.log(`::error::${error.message}`); + return 1; + } +} + +if (require.main === module) { + const [name, ...args] = process.argv.slice(2); + runStep(name, args, actions).then(code => process.exit(code)); +} + +module.exports = { actions, runStep }; diff --git a/tools/release-tag.jasmine.ts b/tools/release-tag.jasmine.ts new file mode 100644 index 000000000..e31738594 --- /dev/null +++ b/tools/release-tag.jasmine.ts @@ -0,0 +1,131 @@ +import { gt as semverGt } from 'semver'; +import { highestReleaseAbove, movesNext, publishedUnder, releasesAbove, releaseTag } from './release-tag.js'; +import 'jasmine'; + +/* npm's dist-tags and the repository's stable tags once 21.0.0 ships. */ +const after21 = { latest: '21.0.0', next: '21.0.0', canary: '21.0.1-canary.20261005034752.sha-bc3fdad' }; +const tagsAfter21 = ['5.1', 'v5.2.0', '20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0']; + +describe('releaseTag', () => { + + it('publishes a new major to latest', () => { + const today = { latest: '20.1.0', next: '21.0.0-rc.1' }; + expect(releaseTag('21.0.0', today, ['20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0'])).toBe('latest'); + }); + + it('publishes a patch, minor or major above latest to latest', () => { + expect(releaseTag('21.0.1', after21, [...tagsAfter21, '21.0.1'])).toBe('latest'); + expect(releaseTag('21.1.0', after21, [...tagsAfter21, '21.1.0'])).toBe('latest'); + expect(releaseTag('22.0.0', after21, [...tagsAfter21, '22.0.0'])).toBe('latest'); + }); + + it('compares each number, not the version text', () => { + const at21dot9 = { latest: '21.9.0' }; + expect(releaseTag('21.10.0', at21dot9, ['21.9.0', '21.10.0'])).toBe('latest'); + expect(() => releaseTag('21.9.1', { latest: '21.10.0' }, ['21.10.0', '21.9.1'])).toThrowError(/does not rank above 21\.10\.0\.$/); + }); + + it('publishes an older major to its lts tag', () => { + expect(releaseTag('20.1.1', after21, [...tagsAfter21, '20.1.1'])).toBe('v20-lts'); + }); + + it('publishes to the lts tag while a higher tagged release is still in npm\'s scan', () => { + const stillScanning = { latest: '20.1.0', next: '21.0.0-rc.1' }; + expect(releaseTag('20.1.1', stillScanning, [...tagsAfter21, '20.1.1'])).toBe('v20-lts'); + }); + + it('publishes a later patch to an existing lts tag', () => { + const withLts = { ...after21, 'v20-lts': '20.1.1' }; + expect(releaseTag('20.1.2', withLts, [...tagsAfter21, '20.1.1', '20.1.2'])).toBe('v20-lts'); + }); + + it('ignores prerelease tags above the release', () => { + expect(releaseTag('21.0.1', after21, [...tagsAfter21, '21.1.0-rc.0', '22.0.0-next.0', '21.0.1'])).toBe('latest'); + }); + + it('refuses a release below the current lts version', () => { + const withLts = { ...after21, 'v20-lts': '20.1.1' }; + expect(() => releaseTag('20.0.4', withLts, [...tagsAfter21, '20.1.1', '20.0.4'])).toThrowError(/does not rank above 20\.1\.1/); + expect(() => releaseTag('20.1.1', withLts, [...tagsAfter21, '20.1.1'])).toThrowError(/does not rank above 20\.1\.1/); + }); + + it('refuses the first lts release of a major below another release of that major', () => { + expect(() => releaseTag('20.0.4', after21, [...tagsAfter21, '20.0.4'])).toThrowError('Not publishing 20.0.4 under v20-lts, because it does not rank above 20.1.0.'); + }); + + it('publishes every prerelease to next', () => { + expect(releaseTag('21.1.0-rc.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('20.2.0-rc.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('22.0.0-next.0', after21, tagsAfter21)).toBe('next'); + expect(releaseTag('22.0.0-alpha.1', after21, tagsAfter21)).toBe('next'); + }); + + it('refuses versions it cannot read', () => { + expect(() => releaseTag('21.0', after21, tagsAfter21)).toThrowError(/not a semver version/); + expect(() => releaseTag('21.0.0-', after21, tagsAfter21)).toThrowError(/not a semver version/); + }); + + it('ranks stable versions exactly as semver does', () => { + const others = ['20.9.9', '21.0.0-rc.1', '21.0.0', '21.0.1-rc.0', '21.0.1', '21.0.2', '21.1.0-rc.0', '21.1.0', '21.10.0', '22.0.0-canary.1']; + for (const version of ['21.0.0', '21.0.1', '21.1.0', '21.9.0', '21.10.0']) { + for (const other of others) { + const expected = semverGt(version, other) ? 'latest' : 'v21-lts'; + expect(`${version} over ${other}: ${releaseTag(version, { latest: other }, [version])}`) + .toBe(`${version} over ${other}: ${expected}`); + } + } + }); +}); + +describe('movesNext', () => { + + // By the time this runs, npm's `latest` is already the release. + it('moves next up to a stable release', () => { + expect(movesNext('21.0.0', { latest: '21.0.0', next: '21.0.0-rc.1' })).toBeTrue(); + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.0.0' })).toBeTrue(); + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.0.1-rc.0' })).toBeTrue(); + }); + + it('moves next when npm has none', () => { + expect(movesNext('21.0.0', { latest: '21.0.0' })).toBeTrue(); + }); + + it('leaves next on a higher version', () => { + expect(movesNext('21.0.1', { latest: '21.0.1', next: '21.1.0-rc.0' })).toBeFalse(); + expect(movesNext('21.0.0', { latest: '21.0.0', next: '21.0.0' })).toBeFalse(); + }); + + it('never moves next to a prerelease', () => { + expect(movesNext('21.1.0-rc.0', { latest: '21.0.0', next: '21.0.0' })).toBeFalse(); + }); +}); + +describe('publishedUnder', () => { + + it('is undefined for a version npm does not list', () => { + expect(publishedUnder('21.0.1', after21)).toBeUndefined(); + }); + + it('names the dist-tag that holds the version, preferring latest', () => { + expect(publishedUnder('21.0.0', { next: '21.0.0', latest: '21.0.0' })).toBe('latest'); + expect(publishedUnder('20.1.1', { latest: '21.0.0', 'v20-lts': '20.1.1' })).toBe('v20-lts'); + }); +}); + +describe('highestReleaseAbove', () => { + + it('names only the highest release the version does not rank above', () => { + const tags = [...tagsAfter21, '22.0.0', '22.6.3', '22.1.0', '21.0.1']; + expect(highestReleaseAbove('21.0.1', { latest: '22.0.0' }, tags)).toBe('22.6.3'); + expect(highestReleaseAbove('20.1.1', { latest: '20.1.0' }, [...tagsAfter21, '21.1.0', '20.1.1'])).toBe('21.1.0'); + }); +}); + +describe('releasesAbove', () => { + + it('lists npm\'s latest and the stable git tags the release does not rank above, once each', () => { + expect(releasesAbove('20.1.1', after21, [...tagsAfter21, '20.1.1'])).toEqual(['21.0.0']); + expect(releasesAbove('21.0.2', after21, [...tagsAfter21, '21.1.0', '21.0.2'])).toEqual(['21.1.0']); + expect(releasesAbove('21.0.1', after21, [...tagsAfter21, '21.1.0-rc.0', '21.0.1'])).toEqual([]); + }); +}); diff --git a/tools/release-tag.js b/tools/release-tag.js new file mode 100644 index 000000000..94eb46d98 --- /dev/null +++ b/tools/release-tag.js @@ -0,0 +1,82 @@ +// No dependencies: the publish job runs this without installing node_modules. +const STABLE = /^v?(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$/; +const ANY = /^v?(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$/; + +/** Splits a version or tag into its three numbers and whether it is a prerelease. Throws if it is not semver. */ +function parse(version) { + const match = ANY.exec(version); + if (!match) { + throw new Error(`${version} is not a semver version.`); + } + return { numbers: match.slice(1, 4).map(Number), isPrerelease: !!match[4] }; +} + +/** Whether the stable `version` ranks above `other` in semver order. Only exact for a stable `version`. */ +function isAbove(version, other) { + const [mine, theirs] = [parse(version), parse(other)]; + for (let i = 0; i < 3; i++) { + if (mine.numbers[i] !== theirs.numbers[i]) { + return mine.numbers[i] > theirs.numbers[i]; + } + } + return theirs.isPrerelease; +} + +/** Whether `tag` and `version` have the same three numbers, ignoring a `v` prefix and any prerelease part. */ +const sameRelease = (tag, version) => parse(tag).numbers.join('.') === parse(version).numbers.join('.'); + +/** + * Picks the npm dist-tag a tagged release publishes under. + * + * A stable version goes to `latest` only if it ranks above both npm's `latest` and every other + * stable git tag, since a higher release can be tagged and still be in npm's publish-time malware scan. + * Any other stable version goes to `v-lts`, and must rank above that tag's current version. + * + * @param {string} version The version being published. + * @param {Record} distTags npm's current dist-tags. + * @param {string[]} gitTags Every tag name in the repository. + * @returns {string} The dist-tag. Throws if the version must not be published. + */ +function releaseTag(version, distTags, gitTags) { + if (!STABLE.test(version)) { + parse(version); + return 'next'; + } + if (releasesAbove(version, distTags, gitTags).length === 0) { + return 'latest'; + } + const major = parse(version).numbers[0]; + const ltsTag = `v${major}-lts`; + const sameMajorReleases = gitTags.filter(tag => STABLE.test(tag) && !sameRelease(tag, version) && parse(tag).numbers[0] === major); + const blocking = [distTags[ltsTag], ...sameMajorReleases].filter(release => release && !isAbove(version, release)); + if (blocking.length) { + throw new Error(`Not publishing ${version} under ${ltsTag}, because it does not rank above ${[...new Set(blocking)].join(', ')}.`); + } + return ltsTag; +} + +/** + * The dist-tag npm already lists `version` under, preferring `latest`, which means this run is a + * re-run of a published release. + */ +function publishedUnder(version, distTags) { + return distTags.latest === version ? 'latest' : Object.keys(distTags).find(tag => distTags[tag] === version); +} + +/** The highest of npm's `latest` and the other stable git tags that `version` does not rank above. */ +function highestReleaseAbove(version, distTags, gitTags) { + return releasesAbove(version, distTags, gitTags).reduce((highest, release) => (isAbove(release, highest) ? release : highest)); +} + +/** npm's `latest` and the other stable git tags that `version` does not rank above. */ +function releasesAbove(version, distTags, gitTags) { + const otherReleases = gitTags.filter(tag => STABLE.test(tag) && !sameRelease(tag, version)); + return [...new Set([distTags.latest, ...otherReleases])].filter(release => !isAbove(version, release)); +} + +/** Whether a published release should become `next`: it is stable and above npm's `next`. */ +function movesNext(version, distTags) { + return STABLE.test(version) && (!distTags.next || isAbove(version, distTags.next)); +} + +module.exports = { highestReleaseAbove, movesNext, publishedUnder, releasesAbove, releaseTag }; From 5fd36b3872e5356a3f88982d9a9929b1356bb47f Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Wed, 7 Oct 2026 16:14:25 -0700 Subject: [PATCH 2/2] refactor(ci): move the canary check into tools/publish-job.js The check that skips a canary of an earlier commit keeps the same decisions and messages, and its specs now cover each branch. It runs only for pushes and scheduled runs, and a failed read of npm's dist-tags now says why. --- .github/workflows/test.yml | 29 ++--------------- tools/publish-job.jasmine.ts | 63 +++++++++++++++++++++++++++++++++++- tools/publish-job.js | 41 +++++++++++++++++++++++ 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0aa4ca3d5..4f9715239 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -192,33 +192,8 @@ jobs: path: dist - name: Skip a canary that is not newer than npm's id: canary_check - run: | - VERSION=$(node -p "require('./dist/packages-dist/package.json').version") - if [[ $VERSION == *-canary.* ]]; then - # The dist-tags endpoint is not CDN-cached, unlike the package data `npm view` reads. - DIST_TAGS=$(curl -fsS --retry 3 --max-time 30 https://registry.npmjs.org/-/package/@angular/fire/dist-tags) - NPM_CANARY=$(node -p "JSON.parse(process.argv[1]).canary" "$DIST_TAGS") - # Order by position on main, not by version, which can be higher for an older commit. - git clone --quiet --bare --filter=tree:0 "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" history.git - HOW_TO_FIX="Every canary publish fails until the canary dist-tag points at a build of a commit on main. Publish rights are required to fix it with: npm dist-tag add @angular/fire@ canary" - if ! NPM_CANARY_COMMIT=$(git -C history.git rev-parse --verify --quiet "${NPM_CANARY##*[.-]}^{commit}"); then - echo "::error::Could not match the canary on npm, $NPM_CANARY, to a single commit in this repository. $HOW_TO_FIX" - exit 1 - fi - # `npm publish` always moves a dist-tag, so a canary that is not newer must not publish at all. - if [[ $NPM_CANARY_COMMIT == "$GITHUB_SHA" ]]; then - if [[ $VERSION == "$NPM_CANARY" ]]; then - echo "::notice::Not publishing $VERSION, because it is already the canary on npm." - echo "skip=true" >> "$GITHUB_OUTPUT" - fi - elif git -C history.git merge-base --is-ancestor "$GITHUB_SHA" "$NPM_CANARY_COMMIT"; then - echo "::warning::Not publishing $VERSION, because the canary on npm, $NPM_CANARY, is from a later commit on main." - echo "skip=true" >> "$GITHUB_OUTPUT" - elif ! git -C history.git merge-base --is-ancestor "$NPM_CANARY_COMMIT" "$GITHUB_SHA"; then - echo "::error::Not publishing $VERSION, because its commit and the commit of the canary on npm, $NPM_CANARY, are not on the same line of history. One of them is not on main. $HOW_TO_FIX" - exit 1 - fi - fi + if: github.event_name != 'release' + run: node tools/publish-job.js canary-check - name: Choose the release's dist-tag id: release_tag if: github.event_name == 'release' diff --git a/tools/publish-job.jasmine.ts b/tools/publish-job.jasmine.ts index 5b3f2286c..0bd7145ab 100644 --- a/tools/publish-job.jasmine.ts +++ b/tools/publish-job.jasmine.ts @@ -15,6 +15,9 @@ interface FakeActionsOptions { // Leave out to make any read of the repository's tags fail the spec. gitTags?: string[]; npmFails?: boolean; + // The commit being published, and main's history as each commit's full list of ancestors. + commit?: string; + history?: Record; } interface FakeRecord { @@ -28,7 +31,7 @@ const unreadable = (url: string) => new Error(`Could not read ${url}: fetch fail const distTagsUrl = 'https://registry.npmjs.org/-/package/@angular/fire/dist-tags'; /** A publish job with no network, no git, no npm and a clock that only `sleep` moves. */ -function fakeActions({ version, distTagReads, gitTags, npmFails = false }: FakeActionsOptions) { +function fakeActions({ version, distTagReads, gitTags, npmFails = false, commit = '', history = {} }: FakeActionsOptions) { let clock = 0; const reads = [...distTagReads]; const record: FakeRecord = { logs: [], outputs: {}, commands: [], sleeps: 0 }; @@ -47,6 +50,16 @@ function fakeActions({ version, distTagReads, gitTags, npmFails = false }: FakeA } return gitTags; }, + commit: () => commit, + cloneHistory: () => undefined, + resolveCommit: (abbreviation: string) => { + const matches = Object.keys(history).filter(sha => sha.startsWith(abbreviation)); + if (matches.length !== 1) { + throw new Error('fatal: Needed a single revision'); + } + return matches[0]; + }, + isAncestor: (ancestor: string, descendant: string) => ancestor === descendant || (history[descendant] ?? []).includes(ancestor), run: (command: string, args: string[]) => { record.commands.push([command, ...args].join(' ')); if (npmFails) { @@ -71,6 +84,46 @@ function fakeActions({ version, distTagReads, gitTags, npmFails = false }: FakeA const tagsAfter21 = ['20.0.3', '20.1.0', '21.0.0-rc.1', '21.0.0']; const moveNextCommand = 'npm dist-tag add @angular/fire@21.0.0 next --registry https://wombat-dressing-room.appspot.com'; +describe('publish-job.js canary-check', () => { + // main: aaaaaaa1, then bbbbbbb2, then ccccccc3. ddddddd4 is on no line of main's history. + const history = { aaaaaaa1: [], bbbbbbb2: ['aaaaaaa1'], ccccccc3: ['aaaaaaa1', 'bbbbbbb2'], ddddddd4: [] }; + const canaryOf = (sha: string) => `21.0.1-canary.20261001000000.sha-${sha.slice(0, 7)}`; + const check = (commit: string, npmCanary: string) => + fakeActions({ version: canaryOf(commit), distTagReads: [{ canary: npmCanary }], commit, history }); + + it('publishes a canary of a later commit', async () => { + const { actions, record } = check('ccccccc3', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], actions)).toBe(0); + expect([record.outputs, record.logs]).toEqual([{}, []]); + }); + + it('skips a canary of an earlier commit', async () => { + const { actions, record } = check('aaaaaaa1', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], actions)).toBe(0); + expect(record.outputs).toEqual({ skip: 'true' }); + expect(record.logs).toEqual([`::warning::Not publishing ${canaryOf('aaaaaaa1')}, because the canary on npm, ${canaryOf('bbbbbbb2')}, is from a later commit on main.`]); + }); + + it('skips the canary already on npm, and publishes another version of the same commit', async () => { + const same = check('bbbbbbb2', canaryOf('bbbbbbb2')); + expect(await publishJob.runStep('canary-check', [], same.actions)).toBe(0); + expect(same.record.outputs).toEqual({ skip: 'true' }); + expect(same.record.logs[0]).toMatch(/^::notice::Not publishing .* because it is already the canary on npm\.$/); + const renamed = check('bbbbbbb2', '21.0.0-canary.bbbbbbb'); + expect(await publishJob.runStep('canary-check', [], renamed.actions)).toBe(0); + expect(renamed.record.outputs).toEqual({}); + }); + + it('fails when the canary on npm is not a commit on main\'s line', async () => { + const unrelated = check('ccccccc3', canaryOf('ddddddd4')); + expect(await publishJob.runStep('canary-check', [], unrelated.actions)).toBe(1); + expect(unrelated.record.logs[0]).toMatch(/^::error::Not publishing .* are not on the same line of history\. One of them is not on main\. Every canary publish fails/); + const unknown = check('ccccccc3', canaryOf('eeeeeee5')); + expect(await publishJob.runStep('canary-check', [], unknown.actions)).toBe(1); + expect(unknown.record.logs[0]).toMatch(/^::error::Could not match the canary on npm, .*sha-eeeeeee, to a single commit in this repository\./); + }); +}); + describe('publish-job.js release-tag', () => { it('publishes a new major to latest', async () => { @@ -227,6 +280,14 @@ describe('publish-job.js actions', () => { expect(publishJob.actions.gitTags().sort()).toEqual(['21.0.0', '21.0.1']); }); + it('resolves abbreviations and orders commits in the cloned history', () => { + publishJob.actions.cloneHistory(); + const [one, two, three, unrelated] = commits; + expect(publishJob.actions.resolveCommit(three.slice(0, 7))).toBe(three); + expect(() => publishJob.actions.resolveCommit('0000000')).toThrow(); + expect([publishJob.actions.isAncestor(one, three), publishJob.actions.isAncestor(three, two), publishJob.actions.isAncestor(one, unrelated)]).toEqual([true, false, false]); + }); + it('appends step outputs as name=value lines', () => { writeFileSync(process.env.GITHUB_OUTPUT ?? '', ''); publishJob.actions.setOutput('tag', 'latest'); diff --git a/tools/publish-job.js b/tools/publish-job.js index d3101f526..92b928419 100644 --- a/tools/publish-job.js +++ b/tools/publish-job.js @@ -38,12 +38,52 @@ const actions = { gitTags: () => git(['ls-remote', '--tags', '--refs', repositoryUrl()], { encoding: 'utf8' }) .split('\n').filter(line => line).map(line => line.replace(/.*refs\/tags\//, '')), run: (command, args) => execFileSync(command, args, { stdio: 'inherit' }), + commit: () => process.env.GITHUB_SHA, + cloneHistory: () => git(['clone', '--quiet', '--bare', '--filter=tree:0', repositoryUrl(), 'history.git']), + resolveCommit: abbreviation => git(['-C', 'history.git', 'rev-parse', '--verify', '--quiet', `${abbreviation}^{commit}`], { encoding: 'utf8' }).trim(), + isAncestor: (ancestor, descendant) => { + try { + git(['-C', 'history.git', 'merge-base', '--is-ancestor', ancestor, descendant]); + return true; + } catch { + return false; + } + }, sleep: seconds => new Promise(resolve => setTimeout(resolve, seconds * 1000)), now: () => Date.now(), setOutput: (name, value) => appendFileSync(process.env.GITHUB_OUTPUT, `${name}=${value}\n`), log: message => console.log(message), }; +/** + * Sets the step output `skip` for a canary whose commit is not after the commit of the canary on + * npm, since `npm publish` always moves the dist-tag. Ordered by position on main, not by version, + * which can be higher for an older commit. + */ +async function checkCanary(actions) { + const version = actions.version(); + const npmCanary = (await actions.fetchDistTags()).canary; + actions.cloneHistory(); + const commit = actions.commit(); + const howToFix = 'Every canary publish fails until the canary dist-tag points at a build of a commit on main. Publish rights are required to fix it with: npm dist-tag add @angular/fire@ canary'; + + let npmCanaryCommit; + try { npmCanaryCommit = actions.resolveCommit(npmCanary.split(/[.-]/).pop()); } + catch { throw new Error(`Could not match the canary on npm, ${npmCanary}, to a single commit in this repository. ${howToFix}`); } + + if (npmCanaryCommit === commit) { + if (version === npmCanary) { + actions.log(`::notice::Not publishing ${version}, because it is already the canary on npm.`); + actions.setOutput('skip', 'true'); + } + } else if (actions.isAncestor(commit, npmCanaryCommit)) { + actions.log(`::warning::Not publishing ${version}, because the canary on npm, ${npmCanary}, is from a later commit on main.`); + actions.setOutput('skip', 'true'); + } else if (!actions.isAncestor(npmCanaryCommit, commit)) { + throw new Error(`Not publishing ${version}, because its commit and the commit of the canary on npm, ${npmCanary}, are not on the same line of history. One of them is not on main. ${howToFix}`); + } +} + /** * Chooses the dist-tag a tagged release publishes under and sets the step output `tag`. On a re-run * of a release npm already lists, also sets `published`, so the job skips `npm publish` and goes on @@ -111,6 +151,7 @@ async function moveNext(actions) { } const steps = { + 'canary-check': actions => checkCanary(actions), 'release-tag': actions => chooseReleaseTag(actions), 'wait': (actions, tag) => waitUntilListed(actions, tag), 'move-next': actions => moveNext(actions),