From 1474c5204aead94a0c204b628e01f9f363f1afbe Mon Sep 17 00:00:00 2001 From: SivanCola Date: Tue, 4 Aug 2026 21:18:50 +0800 Subject: [PATCH] fix(release): tolerate forbidden npm staging cleanup (#7477) --- npm/publish.mjs | 12 +++++++++- npm/publish.test.mjs | 18 +++++++++++++- scripts/finalize-npm-official-release.mjs | 16 ++++++++++++- .../finalize-npm-official-release.test.mjs | 24 ++++++++++++++++++- 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/npm/publish.mjs b/npm/publish.mjs index bf3de01f31..8c7ae407e8 100644 --- a/npm/publish.mjs +++ b/npm/publish.mjs @@ -276,7 +276,17 @@ function cleanupStagingTag(runner, name, version, stagingTag, log) { const current = readDistTag(runner, name, stagingTag); if (current !== version) return; log(`remove temporary ${name} dist-tag ${stagingTag}`); - runner(["dist-tag", "rm", name, stagingTag], { inherit: true }); + try { + // Capture stderr for this best-effort cleanup so an npm E403 can be + // distinguished from transport, authentication, and registry failures. + runner(["dist-tag", "rm", name, stagingTag]); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + if (!/\bE403\b|\b403 Forbidden\b/.test(detail)) throw error; + log( + `keep temporary ${name} dist-tag ${stagingTag}: npm refused cleanup with E403`, + ); + } } export function publishPackages({ diff --git a/npm/publish.test.mjs b/npm/publish.test.mjs index 77fcacea8c..cd56c12cc7 100644 --- a/npm/publish.test.mjs +++ b/npm/publish.test.mjs @@ -16,7 +16,7 @@ function splitPackageSpec(spec) { return [spec.slice(0, separator), spec.slice(separator + 1)]; } -function fixture(t, version = "1.5.0-canary.42") { +function fixture(t, version = "1.5.0-canary.42", { forbidCleanup = false } = {}) { const root = mkdtempSync(join(tmpdir(), "reasonix-npm-publish-test-")); t.after(() => rmSync(root, { recursive: true, force: true })); @@ -76,6 +76,9 @@ function fixture(t, version = "1.5.0-canary.42") { return ""; } if (args[0] === "dist-tag" && args[1] === "rm") { + if (forbidCleanup) { + throw new Error("npm dist-tag failed with exit code 1: npm error code E403"); + } packageState(args[2]).tags.delete(args[3]); return ""; } @@ -192,6 +195,19 @@ test("accepts legacy packages whose gitHead proves the candidate", (t) => { assert.doesNotThrow(() => fx.publish()); }); +test("does not fail a completed publish when npm forbids staging cleanup", (t) => { + const fx = fixture(t, "1.5.0-canary.42", { forbidCleanup: true }); + + assert.doesNotThrow(() => fx.publish()); + for (const { name } of fx.packages) { + assert.equal(fx.registry.get(name).tags.get("canary"), "1.5.0-canary.42"); + assert.equal( + fx.registry.get(name).tags.get("canary-staging"), + "1.5.0-canary.42", + ); + } +}); + test("compares channel versions without integer truncation", () => { assert.equal( compareDistTagVersions( diff --git a/scripts/finalize-npm-official-release.mjs b/scripts/finalize-npm-official-release.mjs index 8a38605aa2..d046a9a565 100644 --- a/scripts/finalize-npm-official-release.mjs +++ b/scripts/finalize-npm-official-release.mjs @@ -48,6 +48,20 @@ function distTags(name) { return JSON.parse(execFileSync("npm", ["view", name, "dist-tags", "--json"], { encoding: "utf8" })); } +function removeStagingTag(name) { + try { + execFileSync("npm", ["dist-tag", "rm", name, stagingTag], { encoding: "utf8" }); + } catch (error) { + const detail = [error?.message, error?.stdout, error?.stderr] + .filter(Boolean) + .join("\n"); + if (!/\bE403\b|\b403 Forbidden\b/.test(detail)) throw error; + console.warn( + `npm refused cleanup of ${name} dist-tag ${stagingTag} with E403; official aliases are already verified`, + ); + } +} + async function waitForOfficialAliases(name) { let tags = {}; for (let attempt = 1; attempt <= verifyAttempts; attempt += 1) { @@ -88,6 +102,6 @@ for (const name of packages) { } const tags = await waitForOfficialAliases(name); if (tags[stagingTag] === version) { - execFileSync("npm", ["dist-tag", "rm", name, stagingTag], { stdio: "inherit" }); + removeStagingTag(name); } } diff --git a/scripts/finalize-npm-official-release.test.mjs b/scripts/finalize-npm-official-release.test.mjs index 36336fc30b..9d46a345cb 100644 --- a/scripts/finalize-npm-official-release.test.mjs +++ b/scripts/finalize-npm-official-release.test.mjs @@ -10,7 +10,12 @@ const candidate = "c46e3af1c2732fe2b3dedb0bd47eb39a629357d2"; // The fake npm records every dist-tag mutation so a test can assert not just // the outcome but how many registry writes it took to get there — the whole // point of the idempotent path is that a rerun performs none. -function run(expectedSha = candidate, publishedSha = candidate, staleReads = 0, { staging = false } = {}) { +function run( + expectedSha = candidate, + publishedSha = candidate, + staleReads = 0, + { staging = false, forbidRemoval = false } = {}, +) { const directory = mkdtempSync(join(tmpdir(), "reasonix-npm-alias-test-")); const npm = join(directory, "npm"); const state = join(directory, "state"); @@ -33,6 +38,11 @@ if (args[0] === "view" && args[1].includes("@1.19.2") && args.at(-1) === "--json console.log(JSON.stringify(tags)); } else if (args[0] === "dist-tag" && (args[1] === "add" || args[1] === "rm")) { fs.appendFileSync(process.env.NPM_FAKE_WRITES, JSON.stringify(args) + "\\n"); + if (args[1] === "rm" && process.env.NPM_FAKE_FORBID_REMOVAL === "1") { + console.error("npm error code E403"); + console.error("npm error 403 Forbidden"); + process.exit(1); + } process.exit(0); } else { console.error("unexpected npm arguments", JSON.stringify(args)); @@ -49,6 +59,7 @@ if (args[0] === "view" && args[1].includes("@1.19.2") && args.at(-1) === "--json NPM_FAKE_STATE: state, NPM_FAKE_STALE_READS: String(staleReads), NPM_FAKE_STAGING: staging ? "1" : "0", + NPM_FAKE_FORBID_REMOVAL: forbidRemoval ? "1" : "0", NPM_FAKE_WRITES: writes, NPM_TAG_VERIFY_DELAY_MS: "1", PATH: `${directory}:${process.env.PATH}`, @@ -109,3 +120,14 @@ test("removes the staging alias the publisher actually creates", () => { assert.equal(args.at(-1), "latest-staging"); } }); + +test("completes when npm forbids removal after official aliases converge", () => { + const result = run(candidate, candidate, 0, { + staging: true, + forbidRemoval: true, + }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /official aliases are already verified/); + const removals = result.writes.filter((args) => args[1] === "rm"); + assert.equal(removals.length, 7); +});