From 54a4e34b47d6bbb0c5c1287bab69b8dc65742738 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 2 Jun 2026 19:25:25 -0400 Subject: [PATCH] fix(#623): make release Verify-publish tolerant of npm propagation lag (#624) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#623): make release Verify-publish tolerant of npm propagation lag The rc and latest Verify-publish steps used a single `sleep 10` + one `npm view`, which false-failed the whole release job when npm's registry read lagged the publish write — observed on the v1.3.0-rc.1 RC run, where publish/tag/GitHub-release all succeeded but verification reported NOT_FOUND. Extract the check into scripts/verify-npm-publish.cjs: a testable module with a bounded retry/poll loop, a frozen REASON enum, and a --json mode (mirrors verify-reapply-patches.cjs). Both workflow steps now call it. dist-tag reporting stays informational and never fails the step, matching prior behavior. Adds tests/verify-npm-publish.test.cjs covering retry, exhaustion, and dist-tag reporting via injected lookups (no network). Co-Authored-By: Claude Opus 4.8 * chore(#623): allowlist verify-npm-publish.test.cjs in the verify cluster The test-file-count linter groups test files by production-module prefix. scripts/verify-npm-publish.cjs lives under scripts/ (not a scanned prod dir), so its test collapses into the existing `verify` module via the startsWith(prefix + '-') rule — same as scripts/verify-reapply-patches.cjs, whose tests are already allowlisted under `verify`. Add the new test to that cluster's allowlisted set to satisfy the identity ratchet. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .github/workflows/release.yml | 24 +- scripts/lint-test-file-count.allowlist.json | 1 + scripts/verify-npm-publish.cjs | 252 ++++++++++++++++++++ tests/verify-npm-publish.test.cjs | 148 ++++++++++++ 4 files changed, 403 insertions(+), 22 deletions(-) create mode 100644 scripts/verify-npm-publish.cjs create mode 100644 tests/verify-npm-publish.test.cjs diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2dc8a087d..e4d7c2141 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -243,17 +243,7 @@ jobs: if: ${{ !inputs.dry_run }} env: PRE_VERSION: ${{ steps.prerelease.outputs.pre_version }} - run: | - sleep 10 - PUBLISHED=$(npm view @opengsd/gsd-core@"$PRE_VERSION" version 2>/dev/null || echo "NOT_FOUND") - if [ "$PUBLISHED" != "$PRE_VERSION" ]; then - echo "::error::Published version verification failed. Expected $PRE_VERSION, got $PUBLISHED" - exit 1 - fi - echo "✓ Verified: @opengsd/gsd-core@$PRE_VERSION is live on npm" - # Also verify dist-tag - NEXT_TAG=$(npm dist-tag ls @opengsd/gsd-core 2>/dev/null | grep "next:" | awk '{print $2}') - echo "✓ next tag points to: $NEXT_TAG" + run: node scripts/verify-npm-publish.cjs --package @opengsd/gsd-core --version "$PRE_VERSION" --dist-tag next - name: Summary env: @@ -407,17 +397,7 @@ jobs: if: ${{ !inputs.dry_run }} env: VERSION: ${{ inputs.version }} - run: | - sleep 10 - PUBLISHED=$(npm view @opengsd/gsd-core@"$VERSION" version 2>/dev/null || echo "NOT_FOUND") - if [ "$PUBLISHED" != "$VERSION" ]; then - echo "::error::Published version verification failed. Expected $VERSION, got $PUBLISHED" - exit 1 - fi - echo "✓ Verified: @opengsd/gsd-core@$VERSION is live on npm" - # Verify latest tag - LATEST_TAG=$(npm dist-tag ls @opengsd/gsd-core 2>/dev/null | grep "latest:" | awk '{print $2}') - echo "✓ latest tag points to: $LATEST_TAG" + run: node scripts/verify-npm-publish.cjs --package @opengsd/gsd-core --version "$VERSION" --dist-tag latest - name: Summary env: diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index b5f412484..2379d2e61 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -109,6 +109,7 @@ "bug-3657-verify-reapply-patches-pristine-drift.test.cjs", "verify-health.test.cjs", "verify-mvp-uat.test.cjs", + "verify-npm-publish.test.cjs", "verify-test-quality.test.cjs", "verify-work-auto-transition.test.cjs", "verify.test.cjs" diff --git a/scripts/verify-npm-publish.cjs b/scripts/verify-npm-publish.cjs new file mode 100644 index 000000000..e0c998a1e --- /dev/null +++ b/scripts/verify-npm-publish.cjs @@ -0,0 +1,252 @@ +#!/usr/bin/env node +'use strict'; + +/** + * verify-npm-publish.cjs — verifies a freshly-published npm version is + * retrievable, tolerating registry/CDN propagation lag via bounded retry. + * Fixes #623. Used by both Verify-publish steps in .github/workflows/release.yml. + */ + +const cp = require('node:child_process'); + +// ---- Constants --------------------------------------------------------------- + +const REASON = Object.freeze({ + OK_VERSION_LIVE: 'ok_version_live', + FAIL_VERSION_NOT_FOUND: 'fail_version_not_found', +}); + +// ---- Sleep ------------------------------------------------------------------- + +const defaultSleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +// ---- npm fetchers ------------------------------------------------------------ + +function defaultFetchVersion(pkg, version) { + try { + const out = cp.execFileSync('npm', ['view', `${pkg}@${version}`, 'version'], + { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + return out || null; + } catch { return null; } +} + +function defaultFetchDistTag(pkg, distTag) { + try { + const out = cp.execFileSync('npm', ['view', pkg, 'dist-tags', '--json'], + { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); + const tags = JSON.parse(out); + return (tags && typeof tags === 'object' && tags[distTag]) || null; + } catch { return null; } +} + +// ---- Core async function (unit-tested seam) ---------------------------------- + +async function verifyPublish({ + pkg, + version, + distTag = null, + fetchVersion = defaultFetchVersion, + fetchDistTag = defaultFetchDistTag, + maxAttempts = 20, + intervalMs = 5000, + sleep = defaultSleep, +}) { + let attempts = 0; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const found = fetchVersion(pkg, version); + attempts++; + + if (found === version) { + // Version confirmed live — optionally resolve dist-tag informally + let distTagResult = null; + + if (distTag && typeof distTag === 'string' && distTag.length > 0) { + let pointsTo = null; + + for (let dt = 1; dt <= maxAttempts; dt++) { + const tagVal = fetchDistTag(pkg, distTag); + if (tagVal !== null) { + pointsTo = tagVal; + break; + } + if (dt < maxAttempts) { + await sleep(intervalMs); + } + } + + distTagResult = { + name: distTag, + points_to: pointsTo, + matches: pointsTo === version, + }; + } + + return { + ok: true, + reason: REASON.OK_VERSION_LIVE, + pkg, + version, + attempts, + distTag: distTagResult, + }; + } + + // Not found yet — sleep before retry (but not after the final attempt) + if (attempt < maxAttempts) { + await sleep(intervalMs); + } + } + + return { + ok: false, + reason: REASON.FAIL_VERSION_NOT_FOUND, + pkg, + version, + attempts, + distTag: null, + }; +} + +// ---- Argument parsing -------------------------------------------------------- + +function parseArgs(argv) { + const opts = { + pkg: null, + version: null, + distTag: null, + maxAttempts: 20, + intervalMs: 5000, + json: false, + }; + + const args = argv.slice(); + while (args.length > 0) { + const arg = args.shift(); + + if (arg === '--help' || arg === '-h') { + process.stdout.write( + 'Usage: node scripts/verify-npm-publish.cjs --package --version [options]\n' + + '\n' + + 'Options:\n' + + ' --package npm package name (required)\n' + + ' --version version to verify (required)\n' + + ' --dist-tag dist-tag to report (optional, informational only)\n' + + ' --max-attempts max retry attempts (default: 20)\n' + + ' --interval-ms ms between retries (default: 5000)\n' + + ' --json emit structured JSON output\n' + + ' --help, -h show this help\n' + ); + process.exit(0); + } else if (arg === '--package') { + const val = args.shift(); + if (!val || val.startsWith('-')) { + process.stderr.write('error: --package requires a value\n'); + process.exit(2); + } + opts.pkg = val; + } else if (arg === '--version') { + const val = args.shift(); + if (!val || val.startsWith('-')) { + process.stderr.write('error: --version requires a value\n'); + process.exit(2); + } + opts.version = val; + } else if (arg === '--dist-tag') { + const val = args.shift(); + if (!val || val.startsWith('-')) { + process.stderr.write('error: --dist-tag requires a value\n'); + process.exit(2); + } + opts.distTag = val; + } else if (arg === '--max-attempts') { + const val = args.shift(); + if (!val || val.startsWith('-')) { + process.stderr.write('error: --max-attempts requires a value\n'); + process.exit(2); + } + const n = parseInt(val, 10); + if (isNaN(n) || n < 1) { + process.stderr.write('error: --max-attempts must be a positive integer\n'); + process.exit(2); + } + opts.maxAttempts = n; + } else if (arg === '--interval-ms') { + const val = args.shift(); + if (!val || val.startsWith('-')) { + process.stderr.write('error: --interval-ms requires a value\n'); + process.exit(2); + } + const n = parseInt(val, 10); + if (isNaN(n) || n < 0) { + process.stderr.write('error: --interval-ms must be a non-negative integer\n'); + process.exit(2); + } + opts.intervalMs = n; + } else if (arg === '--json') { + opts.json = true; + } else { + process.stderr.write(`unknown argument: ${arg}\n`); + process.exit(2); + } + } + + if (!opts.pkg) { + process.stderr.write('error: --package is required\n'); + process.exit(2); + } + if (!opts.version) { + process.stderr.write('error: --version is required\n'); + process.exit(2); + } + + return opts; +} + +// ---- Main entry point -------------------------------------------------------- + +async function main() { + const opts = parseArgs(process.argv.slice(2)); + const result = await verifyPublish({ + pkg: opts.pkg, + version: opts.version, + distTag: opts.distTag, + maxAttempts: opts.maxAttempts, + intervalMs: opts.intervalMs, + }); + + if (opts.json) { + process.stdout.write(JSON.stringify(result, null, 2) + '\n'); + } else { + if (result.ok) { + process.stdout.write( + `✓ Verified: ${result.pkg}@${result.version} is live on npm (after ${result.attempts} attempt(s))\n` + ); + if (result.distTag) { + process.stdout.write(`✓ ${result.distTag.name} tag points to: ${result.distTag.points_to}\n`); + if (!result.distTag.matches) { + process.stdout.write( + `::warning::${result.distTag.name} dist-tag points to ${result.distTag.points_to}, expected ${result.version}\n` + ); + } + } + } else { + process.stdout.write( + `::error::Published version verification failed. ${result.pkg}@${result.version} not found after ${result.attempts} attempt(s)\n` + ); + } + } + + process.exit(result.ok ? 0 : 1); +} + +// ---- Guard ------------------------------------------------------------------- + +if (require.main === module) { + main().catch((err) => { + process.stderr.write(String((err && err.stack) || err) + '\n'); + process.exit(1); + }); +} + +module.exports = { verifyPublish, parseArgs, REASON, defaultFetchVersion, defaultFetchDistTag }; diff --git a/tests/verify-npm-publish.test.cjs b/tests/verify-npm-publish.test.cjs new file mode 100644 index 000000000..5e9c4f0dc --- /dev/null +++ b/tests/verify-npm-publish.test.cjs @@ -0,0 +1,148 @@ +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +const SCRIPT = path.join(__dirname, '..', 'scripts', 'verify-npm-publish.cjs'); +const { verifyPublish, REASON } = require(SCRIPT); + +// ---- Helpers ----------------------------------------------------------------- + +function makeFetchVersion(sequence) { + let i = 0; + return () => sequence[Math.min(i++, sequence.length - 1)]; +} + +function makeSleepSpy() { + let sleeps = 0; + const sleep = async () => { sleeps++; }; + const count = () => sleeps; + return { sleep, count }; +} + +// ---- Tests ------------------------------------------------------------------- + +describe('verifyPublish', () => { + test('returns OK on first attempt when version is already live', async () => { + const { sleep, count } = makeSleepSpy(); + const fetchVersion = makeFetchVersion(['1.3.0-rc.1']); + const fetchDistTag = makeFetchVersion([null]); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, true); + assert.equal(result.reason, REASON.OK_VERSION_LIVE); + assert.equal(result.attempts, 1); + assert.equal(count(), 0); + }); + + test('retries through propagation lag and succeeds once the version appears', async () => { + const { sleep, count } = makeSleepSpy(); + const fetchVersion = makeFetchVersion([null, null, '1.3.0-rc.1']); + const fetchDistTag = makeFetchVersion([null]); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, true); + assert.equal(result.attempts, 3); + assert.equal(count(), 2); + }); + + test('fails after exhausting maxAttempts when version never appears', async () => { + const { sleep, count } = makeSleepSpy(); + const fetchVersion = makeFetchVersion([null]); + const fetchDistTag = makeFetchVersion([null]); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + maxAttempts: 4, + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, false); + assert.equal(result.reason, REASON.FAIL_VERSION_NOT_FOUND); + assert.equal(result.attempts, 4); + assert.equal(count(), 3); + }); + + test('reports dist-tag pointer informationally without affecting ok', async () => { + const { sleep, count } = makeSleepSpy(); + const fetchVersion = makeFetchVersion(['1.3.0-rc.1']); + const fetchDistTag = makeFetchVersion(['1.3.0-rc.1']); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + distTag: 'next', + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, true); + assert.deepEqual(result.distTag, { + name: 'next', + points_to: '1.3.0-rc.1', + matches: true, + }); + void count; + }); + + test('dist-tag mismatch is a warning, not a failure', async () => { + const { sleep } = makeSleepSpy(); + const fetchVersion = makeFetchVersion(['1.3.0-rc.1']); + const fetchDistTag = makeFetchVersion(['1.2.0']); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + distTag: 'latest', + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, true); + assert.equal(result.distTag.matches, false); + assert.equal(result.distTag.points_to, '1.2.0'); + }); + + test('no dist-tag requested yields null distTag', async () => { + const { sleep } = makeSleepSpy(); + const fetchVersion = makeFetchVersion(['1.3.0-rc.1']); + const fetchDistTag = makeFetchVersion([null]); + + const result = await verifyPublish({ + pkg: '@opengsd/gsd-core', + version: '1.3.0-rc.1', + fetchVersion, + fetchDistTag, + sleep, + intervalMs: 0, + }); + + assert.equal(result.ok, true); + assert.equal(result.distTag, null); + }); +});