From 94d02ee9dd3911e3e99b2ad20855cafbc783d620 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 14 Oct 2026 03:06:59 -0400 Subject: [PATCH 1/5] build(release): an open release-hold issue stops the release When releasing main as it stands would harm someone running it, an issue labelled release-hold says what has to happen first. Nothing in the release read such a thing before: 0.21.1 and 0.22.0 were both published while #300, the uncapped retryAfterMs sleep, was open. scripts/release-holds.mjs lists the repository's open issues with the label (pull requests aren't holds: one isn't on main until it merges) and exits 1 naming each, or when the holds can't be read. It runs as npm's preversion hook, so npm version stops before it bumps, commits or tags anything, and as the publish workflow's first step for the pushed tag, which both releasing jobs wait on. The repository is GITHUB_REPOSITORY or the one package.json names; GH_TOKEN or GITHUB_TOKEN is sent when set, and the workflow's build job gets issues: read for it. The tests run the real script against a local stand-in for GitHub's issues endpoint: no hold, a hold beside a labelled pull request, only pull requests, several holds, GITHUB_REPOSITORY and the token, an error answer, and no repository to ask. CONTRIBUTING says when to file a hold and what it stops. --- .github/workflows/publish.yml | 14 +++ CONTRIBUTING.md | 5 ++ package.json | 1 + scripts/release-holds.mjs | 72 +++++++++++++++ test/release-holds.test.ts | 164 ++++++++++++++++++++++++++++++++++ 5 files changed, 256 insertions(+) create mode 100644 scripts/release-holds.mjs create mode 100644 test/release-holds.test.ts diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e484b9b4f..d8adb9440 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -14,6 +14,11 @@ jobs: name: Build & Test runs-on: ubuntu-latest + # issues: read lets the release-hold check list the open holds. + permissions: + contents: read + issues: read + steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: @@ -24,6 +29,15 @@ jobs: with: node-version: 20 + # The same check `npm version` runs first (see CONTRIBUTING.md): an + # open issue labelled release-hold stops the release. Both jobs that + # release need this one, so a held tag reaches neither npm nor the + # GitHub release notes. + - name: Release holds + run: node scripts/release-holds.mjs + env: + GH_TOKEN: ${{ github.token }} + - name: Install dependencies run: npm install diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5c87d8f2..87f67f719 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -140,6 +140,11 @@ never do. consumers run github-clone checkouts, so release notes must exist even when npm publish fails. Version bumps are a maintainer release-time action, not part of feature PRs. +- **Release holds.** When releasing `main` as it stands would harm + someone running it, open an issue that says what has to happen first, + label it `release-hold`, and close it once that's happened. While one is + open, `npm version` stops before it bumps anything, and the publish + workflow refuses the tag. ## Building and testing diff --git a/package.json b/package.json index c9add9db3..748179d4d 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "typecheck": "tsc --noEmit", "pretest": "mkdir -p dist/test/fixtures && cp test/fixtures/*.mjs dist/test/fixtures/", "test": "node --test --test-force-exit dist/test/*.test.js", + "preversion": "node scripts/release-holds.mjs", "version": "node scripts/release-changelog.mjs && git add CHANGELOG.md changelog.d", "prepublishOnly": "npm run build" }, diff --git a/scripts/release-holds.mjs b/scripts/release-holds.mjs new file mode 100644 index 000000000..f93e65889 --- /dev/null +++ b/scripts/release-holds.mjs @@ -0,0 +1,72 @@ +// Runs as npm's `preversion` lifecycle hook (see package.json), before +// `npm version` bumps anything, and again in the publish workflow for the +// pushed tag. Stops the release while the repository has an open issue +// labelled `release-hold`: someone found that releasing `main` as it stands +// would harm whoever runs it, and the issue says what has to happen first. +// +// The repository is GITHUB_REPOSITORY when set (as in GitHub Actions), and +// otherwise the GitHub repository package.json names. A token in GH_TOKEN or +// GITHUB_TOKEN is sent when present; the issues of a public repository read +// without one. If the holds can't be read, the release stops too: publishing +// needs GitHub anyway. +import { readFileSync } from "node:fs"; + +const LABEL = "release-hold"; + +function fail(message) { + console.error(message); + process.exit(1); +} + +function repository() { + if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; + let pkg; + try { + pkg = JSON.parse(readFileSync("package.json", "utf8")); + } catch (error) { + fail(`release-holds: can't read package.json: ${error.message}`); + } + const url = typeof pkg.repository === "string" ? pkg.repository : pkg.repository?.url; + const match = /github\.com[/:]([^/]+\/[^/]+?)(?:\.git)?\/?$/.exec(url ?? ""); + if (!match) { + fail("release-holds: package.json names no GitHub repository, and GITHUB_REPOSITORY is unset"); + } + return match[1]; +} + +const repo = repository(); +const api = (process.env.GITHUB_API_URL ?? "https://api.github.com").replace(/\/$/, ""); +const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; +const where = `https://github.com/${repo}/issues?q=is%3Aissue+is%3Aopen+label%3A${LABEL}`; + +let items; +try { + const response = await fetch( + `${api}/repos/${repo}/issues?labels=${LABEL}&state=open&per_page=100`, + { + headers: { + accept: "application/vnd.github+json", + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + signal: AbortSignal.timeout(30_000), + }, + ); + if (!response.ok) throw new Error(`GitHub answered ${response.status} ${response.statusText}`); + items = await response.json(); + if (!Array.isArray(items)) throw new Error("GitHub's answer wasn't a list of issues"); +} catch (error) { + fail(`release-holds: couldn't read the release holds (${error.message}). Check ${where}, and run this again once GitHub can be reached.`); +} + +// The issues endpoint lists pull requests too. A hold is an issue: a pull +// request isn't on main until it merges. +const holds = items.filter((item) => !item.pull_request); +if (holds.length > 0) { + fail( + [ + `Release held: ${holds.length === 1 ? "an open issue is" : `${holds.length} open issues are`} labelled ${LABEL} in ${repo}.`, + ...holds.map((hold) => ` #${hold.number} ${hold.title}`), + `Resolve each one, or close it if it no longer applies, then release again. ${where}`, + ].join("\n"), + ); +} diff --git a/test/release-holds.test.ts b/test/release-holds.test.ts new file mode 100644 index 000000000..0088d6190 --- /dev/null +++ b/test/release-holds.test.ts @@ -0,0 +1,164 @@ +// Black-box coverage of scripts/release-holds.mjs: each case runs the real +// script against a local stand-in for GitHub's issues endpoint, in a +// throwaway directory with its own package.json, as `npm version` and the +// publish workflow run it. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { createServer, type IncomingHttpHeaders } from "node:http"; +import type { AddressInfo } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const SCRIPT = fileURLToPath( + new URL("../../scripts/release-holds.mjs", import.meta.url), +); + +interface Seen { + url: string; + headers: IncomingHttpHeaders; +} + +/** Serves `body` with `status` for every request, recording each one. */ +async function github( + status: number, + body: unknown, +): Promise<{ url: string; seen: Seen[]; close: () => Promise }> { + const seen: Seen[] = []; + const server = createServer((req, res) => { + seen.push({ url: req.url ?? "", headers: req.headers }); + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + return { + url: `http://127.0.0.1:${port}`, + seen, + close: () => new Promise((resolve) => server.close(() => resolve())), + }; +} + +function project(pkg: Record): string { + const dir = mkdtempSync(join(tmpdir(), "release-holds-")); + writeFileSync(join(dir, "package.json"), JSON.stringify(pkg)); + return dir; +} + +const PKG = { + name: "fixture", + version: "1.0.0", + repository: { type: "git", url: "git+https://github.com/owner/repo.git" }, +}; + +/** Runs the script with only the environment given, so a CI runner's own + * GITHUB_REPOSITORY or token can't reach it. */ +function run( + cwd: string, + env: Record, +): Promise<{ code: number; stderr: string }> { + return new Promise((resolve) => { + execFile( + process.execPath, + [SCRIPT], + { cwd, env: { PATH: process.env.PATH ?? "", ...env } }, + (error, _stdout, stderr) => { + resolve({ code: error ? Number(error.code ?? 1) : 0, stderr }); + }, + ); + }); +} + +test("no open hold: the release goes ahead, asking for the repository package.json names", async () => { + const gh = await github(200, []); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 0, result.stderr); + assert.equal(result.stderr, ""); + assert.equal(gh.seen.length, 1); + assert.equal( + gh.seen[0]!.url, + "/repos/owner/repo/issues?labels=release-hold&state=open&per_page=100", + ); + assert.equal(gh.seen[0]!.headers.authorization, undefined); + } finally { + await gh.close(); + } +}); + +test("an open hold stops the release and names it; a labelled pull request isn't a hold", async () => { + const gh = await github(200, [ + { number: 300, title: "DefaultErrorPolicy sleeps the provider's retryAfterMs uncapped" }, + { number: 301, title: "A pull request", pull_request: { url: "x" } }, + ]); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.match(result.stderr, /^Release held: an open issue is labelled release-hold in owner\/repo\./); + assert.match(result.stderr, /\n #300 DefaultErrorPolicy sleeps the provider's retryAfterMs uncapped\n/); + assert.doesNotMatch(result.stderr, /#301/); + assert.match(result.stderr, /github\.com\/owner\/repo\/issues\?q=/); + } finally { + await gh.close(); + } +}); + +test("only labelled pull requests: the release goes ahead", async () => { + const gh = await github(200, [{ number: 301, title: "A pull request", pull_request: { url: "x" } }]); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 0, result.stderr); + } finally { + await gh.close(); + } +}); + +test("several holds are each named", async () => { + const gh = await github(200, [ + { number: 7, title: "first" }, + { number: 9, title: "second" }, + ]); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.match(result.stderr, /^Release held: 2 open issues are labelled release-hold/); + assert.match(result.stderr, /\n #7 first\n #9 second\n/); + } finally { + await gh.close(); + } +}); + +test("GITHUB_REPOSITORY names the repository over package.json, and a token is sent", async () => { + const gh = await github(200, []); + try { + const result = await run(project({ name: "fixture" }), { + GITHUB_API_URL: gh.url, + GITHUB_REPOSITORY: "other/place", + GH_TOKEN: "t0ken", + }); + assert.equal(result.code, 0, result.stderr); + assert.match(gh.seen[0]!.url, /^\/repos\/other\/place\/issues\?/); + assert.equal(gh.seen[0]!.headers.authorization, "Bearer t0ken"); + } finally { + await gh.close(); + } +}); + +test("holds that can't be read stop the release", async () => { + const gh = await github(500, { message: "unavailable" }); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.match(result.stderr, /^release-holds: couldn't read the release holds \(GitHub answered 500/); + } finally { + await gh.close(); + } +}); + +test("no repository to ask stops the release", async () => { + const result = await run(project({ name: "fixture" }), { GITHUB_API_URL: "http://127.0.0.1:9" }); + assert.equal(result.code, 1); + assert.match(result.stderr, /names no GitHub repository/); +}); From 693459f108ee67c0e37c2923c9ec11f22e0eb83c Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 14 Oct 2026 03:20:05 -0400 Subject: [PATCH 2/5] build(release): holds are the package's home repository's, and a hand publish meets them too From the read of 94d02ee9: - The repository is the one package.json names, so a fork's workflow or a copy asks the package's home; GITHUB_REPOSITORY is used only when package.json names none. - prepublishOnly runs the check, so npm publish from a maintainer's machine stops while a hold is open, as npm version and the workflow do. - The workflow's step runs for a tag only: a dispatch on a branch releases nothing, so an open hold doesn't fail it. - Messages say what a maintainer can do: an unreachable GitHub names the network error's code (or 'no answer within 30 s'); 401 points at the token; 403 and 429 at the token's access or the rate limit; 404 at package.json's repository field or a private repository's token. - Tests: package.json over GITHUB_REPOSITORY, GITHUB_REPOSITORY when package.json names none, a 404, and GitHub out of reach (a port just freed: fetch refuses the discard port as 'bad port' without connecting). --- .github/workflows/publish.yml | 4 +- CONTRIBUTING.md | 4 +- package.json | 2 +- scripts/release-holds.mjs | 79 +++++++++++++++++++++++++---------- test/release-holds.test.ts | 47 ++++++++++++++++++--- 5 files changed, 103 insertions(+), 33 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d8adb9440..99e035b2e 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -32,8 +32,10 @@ jobs: # The same check `npm version` runs first (see CONTRIBUTING.md): an # open issue labelled release-hold stops the release. Both jobs that # release need this one, so a held tag reaches neither npm nor the - # GitHub release notes. + # GitHub release notes. A dispatch on a branch releases nothing, so + # it isn't held. - name: Release holds + if: startsWith(github.ref, 'refs/tags/v') run: node scripts/release-holds.mjs env: GH_TOKEN: ${{ github.token }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87f67f719..286a38d49 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -143,8 +143,8 @@ never do. - **Release holds.** When releasing `main` as it stands would harm someone running it, open an issue that says what has to happen first, label it `release-hold`, and close it once that's happened. While one is - open, `npm version` stops before it bumps anything, and the publish - workflow refuses the tag. + open, `npm version` stops before it bumps anything, and `npm publish` + and the publish workflow refuse the release. ## Building and testing diff --git a/package.json b/package.json index 748179d4d..ae7400f89 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "test": "node --test --test-force-exit dist/test/*.test.js", "preversion": "node scripts/release-holds.mjs", "version": "node scripts/release-changelog.mjs && git add CHANGELOG.md changelog.d", - "prepublishOnly": "npm run build" + "prepublishOnly": "node scripts/release-holds.mjs && npm run build" }, "keywords": [ "connectome", diff --git a/scripts/release-holds.mjs b/scripts/release-holds.mjs index f93e65889..bf734f9ac 100644 --- a/scripts/release-holds.mjs +++ b/scripts/release-holds.mjs @@ -1,14 +1,16 @@ // Runs as npm's `preversion` lifecycle hook (see package.json), before -// `npm version` bumps anything, and again in the publish workflow for the -// pushed tag. Stops the release while the repository has an open issue -// labelled `release-hold`: someone found that releasing `main` as it stands -// would harm whoever runs it, and the issue says what has to happen first. +// `npm version` bumps anything; as `prepublishOnly`, before `npm publish`; +// and in the publish workflow for the pushed tag. Stops the release while the +// repository has an open issue labelled `release-hold`: someone found that +// releasing `main` as it stands would harm whoever runs it, and the issue +// says what has to happen first. // -// The repository is GITHUB_REPOSITORY when set (as in GitHub Actions), and -// otherwise the GitHub repository package.json names. A token in GH_TOKEN or -// GITHUB_TOKEN is sent when present; the issues of a public repository read -// without one. If the holds can't be read, the release stops too: publishing -// needs GitHub anyway. +// The repository is the GitHub repository package.json names, so a fork or a +// copy asks the package's home; GITHUB_REPOSITORY (as in GitHub Actions) is +// used only when package.json names none. A token in GH_TOKEN or GITHUB_TOKEN +// is sent when present; the issues of a public repository read without one. +// If the holds can't be read, the release stops too: publishing needs GitHub +// anyway. import { readFileSync } from "node:fs"; const LABEL = "release-hold"; @@ -19,19 +21,38 @@ function fail(message) { } function repository() { - if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; - let pkg; + let named; + let unreadable; try { - pkg = JSON.parse(readFileSync("package.json", "utf8")); + const pkg = JSON.parse(readFileSync("package.json", "utf8")); + const url = typeof pkg.repository === "string" ? pkg.repository : pkg.repository?.url; + named = /github\.com[/:]([^/]+\/[^/]+?)(?:\.git)?\/?$/.exec(url ?? "")?.[1]; } catch (error) { - fail(`release-holds: can't read package.json: ${error.message}`); + unreadable = error.message; } - const url = typeof pkg.repository === "string" ? pkg.repository : pkg.repository?.url; - const match = /github\.com[/:]([^/]+\/[^/]+?)(?:\.git)?\/?$/.exec(url ?? ""); - if (!match) { - fail("release-holds: package.json names no GitHub repository, and GITHUB_REPOSITORY is unset"); + const repo = named ?? process.env.GITHUB_REPOSITORY; + if (!repo) { + fail( + unreadable + ? `release-holds: can't read package.json (${unreadable}), and GITHUB_REPOSITORY is unset` + : "release-holds: package.json names no GitHub repository, and GITHUB_REPOSITORY is unset", + ); } - return match[1]; + return repo; +} + +/** What a maintainer can do about an answer other than the list. */ +function refusal(status) { + if (status === 401) { + return "GitHub doesn't accept the token in GH_TOKEN or GITHUB_TOKEN; replace it, or unset it for a public repository"; + } + if (status === 403 || status === 429) { + return "the token can't read this repository's issues, or this address has used up GitHub's rate limit; give a token that can, or try again later"; + } + if (status === 404) { + return `no repository ${repo} is visible to this request; check package.json's repository field, or give a token if the repository is private`; + } + return "run this again once GitHub answers normally"; } const repo = repository(); @@ -39,9 +60,9 @@ const api = (process.env.GITHUB_API_URL ?? "https://api.github.com").replace(/\/ const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; const where = `https://github.com/${repo}/issues?q=is%3Aissue+is%3Aopen+label%3A${LABEL}`; -let items; +let response; try { - const response = await fetch( + response = await fetch( `${api}/repos/${repo}/issues?labels=${LABEL}&state=open&per_page=100`, { headers: { @@ -51,11 +72,23 @@ try { signal: AbortSignal.timeout(30_000), }, ); - if (!response.ok) throw new Error(`GitHub answered ${response.status} ${response.statusText}`); +} catch (error) { + const why = error.name === "TimeoutError" + ? "no answer within 30 s" + : error.cause?.code ?? error.cause?.message ?? error.message; + fail(`release-holds: couldn't reach GitHub to read the release holds (${why}). Check ${where}, and run this again once GitHub can be reached.`); +} +if (!response.ok) { + fail(`release-holds: GitHub answered ${`${response.status} ${response.statusText}`.trim()} when asked for the release holds: ${refusal(response.status)}. ${where}`); +} +let items; +try { items = await response.json(); - if (!Array.isArray(items)) throw new Error("GitHub's answer wasn't a list of issues"); } catch (error) { - fail(`release-holds: couldn't read the release holds (${error.message}). Check ${where}, and run this again once GitHub can be reached.`); + items = undefined; +} +if (!Array.isArray(items)) { + fail(`release-holds: GitHub's answer wasn't a list of issues. Check ${where}, and run this again.`); } // The issues endpoint lists pull requests too. A hold is an issue: a pull diff --git a/test/release-holds.test.ts b/test/release-holds.test.ts index 0088d6190..a2a88bad0 100644 --- a/test/release-holds.test.ts +++ b/test/release-holds.test.ts @@ -130,35 +130,70 @@ test("several holds are each named", async () => { } }); -test("GITHUB_REPOSITORY names the repository over package.json, and a token is sent", async () => { +test("package.json names the repository, though GITHUB_REPOSITORY names another, and a token is sent", async () => { + const gh = await github(200, []); + try { + const result = await run(project(PKG), { + GITHUB_API_URL: gh.url, + GITHUB_REPOSITORY: "fork/repo", + GH_TOKEN: "t0ken", + }); + assert.equal(result.code, 0, result.stderr); + assert.match(gh.seen[0]!.url, /^\/repos\/owner\/repo\/issues\?/); + assert.equal(gh.seen[0]!.headers.authorization, "Bearer t0ken"); + } finally { + await gh.close(); + } +}); + +test("GITHUB_REPOSITORY names the repository when package.json names none", async () => { const gh = await github(200, []); try { const result = await run(project({ name: "fixture" }), { GITHUB_API_URL: gh.url, GITHUB_REPOSITORY: "other/place", - GH_TOKEN: "t0ken", }); assert.equal(result.code, 0, result.stderr); assert.match(gh.seen[0]!.url, /^\/repos\/other\/place\/issues\?/); - assert.equal(gh.seen[0]!.headers.authorization, "Bearer t0ken"); } finally { await gh.close(); } }); -test("holds that can't be read stop the release", async () => { +test("an error answer stops the release", async () => { const gh = await github(500, { message: "unavailable" }); try { const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); assert.equal(result.code, 1); - assert.match(result.stderr, /^release-holds: couldn't read the release holds \(GitHub answered 500/); + assert.match(result.stderr, /^release-holds: GitHub answered 500 Internal Server Error when asked for the release holds: run this again once GitHub answers normally\./); } finally { await gh.close(); } }); +test("a repository GitHub won't show stops the release and points at package.json", async () => { + const gh = await github(404, { message: "Not Found" }); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.match(result.stderr, /answered 404 Not Found .*no repository owner\/repo is visible to this request; check package\.json's repository field/); + } finally { + await gh.close(); + } +}); + +test("GitHub out of reach stops the release, naming why", async () => { + // A port that was just free: nothing listens there now. (fetch refuses + // the discard port and others as "bad port" without connecting.) + const gh = await github(200, []); + await gh.close(); + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.match(result.stderr, /^release-holds: couldn't reach GitHub to read the release holds \(E[A-Z]+\)\./); +}); + test("no repository to ask stops the release", async () => { - const result = await run(project({ name: "fixture" }), { GITHUB_API_URL: "http://127.0.0.1:9" }); + const result = await run(project({ name: "fixture" }), { GITHUB_API_URL: "http://127.0.0.1:1" }); assert.equal(result.code, 1); assert.match(result.stderr, /names no GitHub repository/); }); From 97487d586936c889699f497da1442bcf98d3b2bd Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 14 Oct 2026 03:23:43 -0400 Subject: [PATCH 3/5] build(release): the publish job's own hold check sends the job's token prepublishOnly now runs the release-hold check inside the publish job's npm publish too. That job had no GITHUB_TOKEN for it and no issues: read, so the request went out unauthenticated from a shared runner address, where GitHub's limit of 60 an hour could fail the publish after the build job's check had passed. The job gets issues: read, and its Publish step passes GH_TOKEN, so the check there asks as the build job's does. --- .github/workflows/publish.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 99e035b2e..d073fd60c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -62,9 +62,11 @@ jobs: # exchanges it for a publish credential. No long-lived NPM_TOKEN secret. # npm-side config: npmjs.com -> package -> Settings -> Trusted Publishers # -> GitHub Actions, matching the repo + this workflow path. + # issues: read lets prepublishOnly's release-hold check list the holds. permissions: contents: read id-token: write + issues: read steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 @@ -95,6 +97,10 @@ jobs: - name: Publish run: npm publish --access public --provenance + env: + # prepublishOnly checks the release holds again, as the build job + # did, with the same token. + GH_TOKEN: ${{ github.token }} github-release: name: GitHub release notes From e1525a84a00cf412c710f7ac9a2ffc6c3c4f0c15 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 14 Oct 2026 04:29:08 -0400 Subject: [PATCH 4/5] build(release): the hold check after its review: npm's repository forms, every page, both release jobs From the review of 97487d58 (sw-af-holds): - The repository field is read in each form npm reads: a github.com URL, github:owner/repo, or owner/repo. A field that names no GitHub repository the check can ask stops it, saying so, rather than falling back to GITHUB_REPOSITORY, which is used only when package.json names none. - Every page of open labelled items is read, so a hold behind a hundred labelled pull requests still counts (up to 5,000 items, past which it stops and says so). - The GitHub release notes job checks the holds too, with issues: read and the job's token, so a hold filed during the run stops the notes as prepublishOnly stops the npm publish. - The stop message says how to go on where it ran: re-run the workflow for the tag, run npm version again, or run npm publish again. - Refusals: 403 and 429 without a token say to set GH_TOKEN, with one that the token can't read the issues or its rate limit is spent; 404 says which; a 5xx says to retry; another status no longer promises a retry helps. A body that can't be read is named as such. - The message is written synchronously before the exit, so a piped stderr gets it whole. - CONTRIBUTING: someone who can't label asks a maintainer; close a hold that no longer applies; both release jobs refuse; the check also stops when it can't read the holds; npm's --ignore-scripts skips it. - Tests (21): npm itself runs this repository's preversion and prepublishOnly in a throwaway package (npm version stops before the bump with a hold open and bumps without one; npm publish --dry-run stops), each repository form, a field naming no GitHub repository, a package.json that isn't JSON, GITHUB_TOKEN alone, the refusals with and without a token, a 200 that isn't a list, a trailing slash on the API URL, a hold on the second page, the message per caller, and the request asserted for labelled pull requests alone. The throwaway directories are removed. --- .github/workflows/publish.yml | 16 ++- CONTRIBUTING.md | 12 +- scripts/release-holds.mjs | 165 ++++++++++++--------- test/release-holds.test.ts | 261 +++++++++++++++++++++++++++++----- 4 files changed, 346 insertions(+), 108 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d073fd60c..6d7c73991 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -31,9 +31,9 @@ jobs: # The same check `npm version` runs first (see CONTRIBUTING.md): an # open issue labelled release-hold stops the release. Both jobs that - # release need this one, so a held tag reaches neither npm nor the - # GitHub release notes. A dispatch on a branch releases nothing, so - # it isn't held. + # release need this one and check again themselves, so a held tag + # reaches neither npm nor the GitHub release notes. A dispatch on a + # branch releases nothing, so it isn't held. - name: Release holds if: startsWith(github.ref, 'refs/tags/v') run: node scripts/release-holds.mjs @@ -111,14 +111,24 @@ jobs: # publish fails — the two jobs succeed or fail on their own. if: startsWith(github.ref, 'refs/tags/v') + # issues: read lets the release-hold check list the holds. permissions: contents: write + issues: read steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: persist-credentials: false + # Checked again here, as prepublishOnly checks it again in the + # publish job: a hold filed after the build job's check stops these + # notes as it stops the npm publish. + - name: Release holds + run: node scripts/release-holds.mjs + env: + GH_TOKEN: ${{ github.token }} + - name: Mirror changelog section into release notes env: GH_TOKEN: ${{ github.token }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 286a38d49..ed34dc1f6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -141,10 +141,14 @@ never do. when npm publish fails. Version bumps are a maintainer release-time action, not part of feature PRs. - **Release holds.** When releasing `main` as it stands would harm - someone running it, open an issue that says what has to happen first, - label it `release-hold`, and close it once that's happened. While one is - open, `npm version` stops before it bumps anything, and `npm publish` - and the publish workflow refuse the release. + someone running it, open an issue that says what has to happen first + and label it `release-hold`, or ask a maintainer to label it. Close it + once that's happened, or if it no longer applies. While one is open, + `npm version` stops before it bumps anything, `npm publish` refuses, + and the publish workflow refuses both its npm publish and its GitHub + release notes. The check also stops a release when it can't read the + holds from GitHub, and says why. npm's `--ignore-scripts` skips it, as + it skips every lifecycle script. ## Building and testing diff --git a/scripts/release-holds.mjs b/scripts/release-holds.mjs index bf734f9ac..dcbae89e9 100644 --- a/scripts/release-holds.mjs +++ b/scripts/release-holds.mjs @@ -1,105 +1,142 @@ // Runs as npm's `preversion` lifecycle hook (see package.json), before // `npm version` bumps anything; as `prepublishOnly`, before `npm publish`; -// and in the publish workflow for the pushed tag. Stops the release while the -// repository has an open issue labelled `release-hold`: someone found that -// releasing `main` as it stands would harm whoever runs it, and the issue -// says what has to happen first. +// and in both of the publish workflow's releasing jobs for the pushed tag. +// Stops the release while the repository has an open issue labelled +// `release-hold`: someone found that releasing `main` as it stands would harm +// whoever runs it, and the issue says what has to happen first. // -// The repository is the GitHub repository package.json names, so a fork or a -// copy asks the package's home; GITHUB_REPOSITORY (as in GitHub Actions) is -// used only when package.json names none. A token in GH_TOKEN or GITHUB_TOKEN -// is sent when present; the issues of a public repository read without one. -// If the holds can't be read, the release stops too: publishing needs GitHub -// anyway. -import { readFileSync } from "node:fs"; +// The repository is the GitHub repository package.json names, as a URL or in +// npm's shorthand, so a fork or a copy asks the package's home; +// GITHUB_REPOSITORY (as in GitHub Actions) is used only when package.json +// names none. A token in GH_TOKEN or GITHUB_TOKEN is sent when present; the +// issues of a public repository read without one. If the holds can't be read, +// the release stops too, since an open hold can't be ruled out. +import { readFileSync, writeSync } from "node:fs"; const LABEL = "release-hold"; +const PAGE = 100; +const MAX_PAGES = 50; function fail(message) { - console.error(message); + // Written synchronously, so a piped stderr has the whole message before + // the exit. + writeSync(2, `${message}\n`); process.exit(1); } +/** owner/repo from package.json's repository field: a github.com URL, or + * npm's shorthand (github:owner/repo, or owner/repo alone). */ +function githubRepository(field) { + const value = typeof field === "string" ? field : field?.url; + if (typeof value !== "string" || value.trim() === "") return {}; + const url = /github\.com[/:]([^/]+\/[^/]+?)(?:\.git)?\/?$/.exec(value); + if (url) return { repo: url[1] }; + const shorthand = /^(?:github:)?([\w.-]+\/[\w.-]+?)(?:\.git)?$/.exec(value); + if (shorthand) return { repo: shorthand[1] }; + return { unreadable: value }; +} + function repository() { - let named; - let unreadable; + let pkg; try { - const pkg = JSON.parse(readFileSync("package.json", "utf8")); - const url = typeof pkg.repository === "string" ? pkg.repository : pkg.repository?.url; - named = /github\.com[/:]([^/]+\/[^/]+?)(?:\.git)?\/?$/.exec(url ?? "")?.[1]; + pkg = JSON.parse(readFileSync("package.json", "utf8")); } catch (error) { - unreadable = error.message; + if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; + fail(`release-holds: can't read package.json (${error.message}), and GITHUB_REPOSITORY is unset`); } - const repo = named ?? process.env.GITHUB_REPOSITORY; - if (!repo) { - fail( - unreadable - ? `release-holds: can't read package.json (${unreadable}), and GITHUB_REPOSITORY is unset` - : "release-holds: package.json names no GitHub repository, and GITHUB_REPOSITORY is unset", - ); + const named = githubRepository(pkg.repository); + if (named.repo) return named.repo; + if (named.unreadable) { + fail(`release-holds: package.json's repository field (${named.unreadable}) names no GitHub repository this check can ask`); } - return repo; + if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; + fail("release-holds: package.json names no repository, and GITHUB_REPOSITORY is unset"); } +const repo = repository(); +const api = (process.env.GITHUB_API_URL ?? "https://api.github.com").replace(/\/+$/, ""); +const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; +const where = `https://github.com/${repo}/issues?q=is%3Aissue+is%3Aopen+label%3A${LABEL}`; + /** What a maintainer can do about an answer other than the list. */ function refusal(status) { if (status === 401) { return "GitHub doesn't accept the token in GH_TOKEN or GITHUB_TOKEN; replace it, or unset it for a public repository"; } if (status === 403 || status === 429) { - return "the token can't read this repository's issues, or this address has used up GitHub's rate limit; give a token that can, or try again later"; + return token + ? "the token in GH_TOKEN or GITHUB_TOKEN can't read this repository's issues, or its rate limit is spent; use one that can, or try again later" + : "this address has used up GitHub's rate limit for requests without a token; set GH_TOKEN to a token that can read the repository's issues, or try again later"; } if (status === 404) { - return `no repository ${repo} is visible to this request; check package.json's repository field, or give a token if the repository is private`; + return token + ? `no repository ${repo} is visible to the token in GH_TOKEN or GITHUB_TOKEN; check package.json's repository field and the token's access` + : `no repository ${repo} is visible without a token; check package.json's repository field, or set GH_TOKEN if the repository is private`; } - return "run this again once GitHub answers normally"; + if (status >= 500) return "GitHub had an error; run this again once it answers normally"; + return "this check doesn't know that answer; see what GitHub shows"; } -const repo = repository(); -const api = (process.env.GITHUB_API_URL ?? "https://api.github.com").replace(/\/$/, ""); -const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; -const where = `https://github.com/${repo}/issues?q=is%3Aissue+is%3Aopen+label%3A${LABEL}`; - -let response; -try { - response = await fetch( - `${api}/repos/${repo}/issues?labels=${LABEL}&state=open&per_page=100`, - { - headers: { - accept: "application/vnd.github+json", - ...(token ? { authorization: `Bearer ${token}` } : {}), +/** One page of the repository's open issues and pull requests with the label. */ +async function page(number) { + let response; + try { + response = await fetch( + `${api}/repos/${repo}/issues?labels=${LABEL}&state=open&per_page=${PAGE}&page=${number}`, + { + headers: { + accept: "application/vnd.github+json", + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + signal: AbortSignal.timeout(30_000), }, - signal: AbortSignal.timeout(30_000), - }, - ); -} catch (error) { - const why = error.name === "TimeoutError" - ? "no answer within 30 s" - : error.cause?.code ?? error.cause?.message ?? error.message; - fail(`release-holds: couldn't reach GitHub to read the release holds (${why}). Check ${where}, and run this again once GitHub can be reached.`); -} -if (!response.ok) { - fail(`release-holds: GitHub answered ${`${response.status} ${response.statusText}`.trim()} when asked for the release holds: ${refusal(response.status)}. ${where}`); -} -let items; -try { - items = await response.json(); -} catch (error) { - items = undefined; -} -if (!Array.isArray(items)) { - fail(`release-holds: GitHub's answer wasn't a list of issues. Check ${where}, and run this again.`); + ); + } catch (error) { + const why = error.name === "TimeoutError" + ? "no answer within 30 s" + : error.cause?.code ?? error.cause?.message ?? error.message; + fail(`release-holds: couldn't reach GitHub to read the release holds (${why}). Check ${where}, and run this again once GitHub can be reached.`); + } + if (!response.ok) { + fail(`release-holds: GitHub answered ${`${response.status} ${response.statusText}`.trim()} when asked for the release holds: ${refusal(response.status)}. ${where}`); + } + let items; + try { + items = await response.json(); + } catch (error) { + fail(`release-holds: couldn't read GitHub's answer (${error.name === "TimeoutError" ? "no answer within 30 s" : error.message}). Check ${where}, and run this again.`); + } + if (!Array.isArray(items)) { + fail(`release-holds: GitHub's answer wasn't a list of issues. Check ${where}, and run this again.`); + } + return items; } // The issues endpoint lists pull requests too. A hold is an issue: a pull // request isn't on main until it merges. -const holds = items.filter((item) => !item.pull_request); +const holds = []; +for (let number = 1; ; number++) { + if (number > MAX_PAGES) { + fail(`release-holds: more than ${MAX_PAGES * PAGE} open items are labelled ${LABEL} in ${repo}, more than this check reads. ${where}`); + } + const items = await page(number); + holds.push(...items.filter((item) => !item.pull_request)); + if (items.length < PAGE) break; +} + if (holds.length > 0) { + const again = process.env.GITHUB_ACTIONS === "true" + ? "then re-run this workflow for the tag" + : process.env.npm_lifecycle_event === "preversion" + ? "then run npm version again" + : process.env.npm_lifecycle_event === "prepublishOnly" + ? "then run npm publish again" + : "then run this again"; fail( [ `Release held: ${holds.length === 1 ? "an open issue is" : `${holds.length} open issues are`} labelled ${LABEL} in ${repo}.`, ...holds.map((hold) => ` #${hold.number} ${hold.title}`), - `Resolve each one, or close it if it no longer applies, then release again. ${where}`, + `Resolve each one, or close it if it no longer applies, ${again}. ${where}`, ].join("\n"), ); } diff --git a/test/release-holds.test.ts b/test/release-holds.test.ts index a2a88bad0..3ea060352 100644 --- a/test/release-holds.test.ts +++ b/test/release-holds.test.ts @@ -1,11 +1,12 @@ // Black-box coverage of scripts/release-holds.mjs: each case runs the real // script against a local stand-in for GitHub's issues endpoint, in a -// throwaway directory with its own package.json, as `npm version` and the -// publish workflow run it. -import { test } from "node:test"; +// throwaway directory with its own package.json. The last cases run npm +// itself with this repository's preversion and prepublishOnly entries, so +// the wiring that makes `npm version` and `npm publish` stop is tested too. +import { after, test } from "node:test"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { createServer, type IncomingHttpHeaders } from "node:http"; import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; @@ -15,22 +16,25 @@ import { fileURLToPath } from "node:url"; const SCRIPT = fileURLToPath( new URL("../../scripts/release-holds.mjs", import.meta.url), ); +const PACKAGE_JSON = fileURLToPath(new URL("../../package.json", import.meta.url)); interface Seen { url: string; headers: IncomingHttpHeaders; } -/** Serves `body` with `status` for every request, recording each one. */ +/** Answers every request with `status` and `body`, or with what `body` + * gives for the request's URL, recording each request. */ async function github( status: number, - body: unknown, + body: unknown | ((url: string) => unknown), ): Promise<{ url: string; seen: Seen[]; close: () => Promise }> { const seen: Seen[] = []; const server = createServer((req, res) => { - seen.push({ url: req.url ?? "", headers: req.headers }); + const url = req.url ?? ""; + seen.push({ url, headers: req.headers }); res.writeHead(status, { "content-type": "application/json" }); - res.end(JSON.stringify(body)); + res.end(JSON.stringify(typeof body === "function" ? (body as (url: string) => unknown)(url) : body)); }); await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); const { port } = server.address() as AddressInfo; @@ -41,9 +45,16 @@ async function github( }; } -function project(pkg: Record): string { +const made: string[] = []; +after(() => { + for (const dir of made) rmSync(dir, { recursive: true, force: true }); +}); + +/** A throwaway directory with this package.json (or this raw text). */ +function project(pkg: Record | string): string { const dir = mkdtempSync(join(tmpdir(), "release-holds-")); - writeFileSync(join(dir, "package.json"), JSON.stringify(pkg)); + made.push(dir); + writeFileSync(join(dir, "package.json"), typeof pkg === "string" ? pkg : JSON.stringify(pkg)); return dir; } @@ -53,24 +64,25 @@ const PKG = { repository: { type: "git", url: "git+https://github.com/owner/repo.git" }, }; -/** Runs the script with only the environment given, so a CI runner's own - * GITHUB_REPOSITORY or token can't reach it. */ -function run( +const HOLD = { number: 300, title: "DefaultErrorPolicy sleeps the provider's retryAfterMs uncapped" }; + +/** Runs `file` with `args` and only the environment given, so a CI + * runner's own GITHUB_REPOSITORY or token can't reach it. */ +function exec( + file: string, + args: string[], cwd: string, env: Record, ): Promise<{ code: number; stderr: string }> { return new Promise((resolve) => { - execFile( - process.execPath, - [SCRIPT], - { cwd, env: { PATH: process.env.PATH ?? "", ...env } }, - (error, _stdout, stderr) => { - resolve({ code: error ? Number(error.code ?? 1) : 0, stderr }); - }, - ); + execFile(file, args, { cwd, env: { PATH: process.env.PATH ?? "", ...env } }, (error, _stdout, stderr) => { + resolve({ code: error ? Number(error.code ?? 1) : 0, stderr }); + }); }); } +const run = (cwd: string, env: Record) => exec(process.execPath, [SCRIPT], cwd, env); + test("no open hold: the release goes ahead, asking for the repository package.json names", async () => { const gh = await github(200, []); try { @@ -80,7 +92,7 @@ test("no open hold: the release goes ahead, asking for the repository package.js assert.equal(gh.seen.length, 1); assert.equal( gh.seen[0]!.url, - "/repos/owner/repo/issues?labels=release-hold&state=open&per_page=100", + "/repos/owner/repo/issues?labels=release-hold&state=open&per_page=100&page=1", ); assert.equal(gh.seen[0]!.headers.authorization, undefined); } finally { @@ -89,27 +101,25 @@ test("no open hold: the release goes ahead, asking for the repository package.js }); test("an open hold stops the release and names it; a labelled pull request isn't a hold", async () => { - const gh = await github(200, [ - { number: 300, title: "DefaultErrorPolicy sleeps the provider's retryAfterMs uncapped" }, - { number: 301, title: "A pull request", pull_request: { url: "x" } }, - ]); + const gh = await github(200, [HOLD, { number: 301, title: "A pull request", pull_request: { url: "x" } }]); try { const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); assert.equal(result.code, 1); assert.match(result.stderr, /^Release held: an open issue is labelled release-hold in owner\/repo\./); assert.match(result.stderr, /\n #300 DefaultErrorPolicy sleeps the provider's retryAfterMs uncapped\n/); assert.doesNotMatch(result.stderr, /#301/); - assert.match(result.stderr, /github\.com\/owner\/repo\/issues\?q=/); + assert.match(result.stderr, /then run this again\. https:\/\/github\.com\/owner\/repo\/issues\?q=/); } finally { await gh.close(); } }); -test("only labelled pull requests: the release goes ahead", async () => { +test("only labelled pull requests: the release goes ahead, after asking", async () => { const gh = await github(200, [{ number: 301, title: "A pull request", pull_request: { url: "x" } }]); try { const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); assert.equal(result.code, 0, result.stderr); + assert.equal(gh.seen.length, 1); } finally { await gh.close(); } @@ -130,7 +140,20 @@ test("several holds are each named", async () => { } }); -test("package.json names the repository, though GITHUB_REPOSITORY names another, and a token is sent", async () => { +test("a hold behind a full page of labelled pull requests still stops the release", async () => { + const pulls = Array.from({ length: 100 }, (_, i) => ({ number: 1000 + i, title: "pr", pull_request: {} })); + const gh = await github(200, (url: string) => (url.endsWith("&page=1") ? pulls : [HOLD])); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); + assert.equal(result.code, 1); + assert.deepEqual(gh.seen.map((s) => s.url.replace(/.*&page=/, "")), ["1", "2"]); + assert.match(result.stderr, /\n #300 /); + } finally { + await gh.close(); + } +}); + +test("package.json names the repository, though GITHUB_REPOSITORY names another, and GH_TOKEN is sent", async () => { const gh = await github(200, []); try { const result = await run(project(PKG), { @@ -146,6 +169,49 @@ test("package.json names the repository, though GITHUB_REPOSITORY names another, } }); +test("GITHUB_TOKEN alone is sent too", async () => { + const gh = await github(200, []); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url, GITHUB_TOKEN: "act1ons" }); + assert.equal(result.code, 0, result.stderr); + assert.equal(gh.seen[0]!.headers.authorization, "Bearer act1ons"); + } finally { + await gh.close(); + } +}); + +test("each form npm reads in the repository field names the same repository", async () => { + const forms: unknown[] = [ + "github:owner/repo", + "owner/repo", + "https://github.com/owner/repo", + "git@github.com:owner/repo.git", + { type: "git", url: "git+ssh://git@github.com/owner/repo.git" }, + ]; + const gh = await github(200, []); + try { + for (const repository of forms) { + const result = await run(project({ name: "fixture", repository }), { + GITHUB_API_URL: gh.url, + GITHUB_REPOSITORY: "fork/repo", + }); + assert.equal(result.code, 0, `${JSON.stringify(repository)}: ${result.stderr}`); + assert.match(gh.seen.at(-1)!.url, /^\/repos\/owner\/repo\/issues\?/, JSON.stringify(repository)); + } + } finally { + await gh.close(); + } +}); + +test("a repository field that names no GitHub repository stops the release, whatever GITHUB_REPOSITORY says", async () => { + const result = await run(project({ name: "fixture", repository: "gitlab:owner/repo" }), { + GITHUB_API_URL: "http://127.0.0.1:1", + GITHUB_REPOSITORY: "fork/repo", + }); + assert.equal(result.code, 1); + assert.match(result.stderr, /package\.json's repository field \(gitlab:owner\/repo\) names no GitHub repository this check can ask/); +}); + test("GITHUB_REPOSITORY names the repository when package.json names none", async () => { const gh = await github(200, []); try { @@ -160,31 +226,58 @@ test("GITHUB_REPOSITORY names the repository when package.json names none", asyn } }); -test("an error answer stops the release", async () => { +test("a package.json that isn't JSON stops the release, saying so", async () => { + const result = await run(project("{ not json"), { GITHUB_API_URL: "http://127.0.0.1:1" }); + assert.equal(result.code, 1); + assert.match(result.stderr, /^release-holds: can't read package\.json \(.+\), and GITHUB_REPOSITORY is unset/); +}); + +test("a server error stops the release, with a retry", async () => { const gh = await github(500, { message: "unavailable" }); try { const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); assert.equal(result.code, 1); - assert.match(result.stderr, /^release-holds: GitHub answered 500 Internal Server Error when asked for the release holds: run this again once GitHub answers normally\./); + assert.match(result.stderr, /^release-holds: GitHub answered 500 Internal Server Error when asked for the release holds: GitHub had an error; run this again once it answers normally\./); } finally { await gh.close(); } }); -test("a repository GitHub won't show stops the release and points at package.json", async () => { - const gh = await github(404, { message: "Not Found" }); +test("each refusal says what to do, with and without a token", async () => { + const cases: Array<{ status: number; env: Record; says: RegExp }> = [ + { status: 401, env: { GH_TOKEN: "stale" }, says: /answered 401 Unauthorized .*doesn't accept the token in GH_TOKEN or GITHUB_TOKEN/ }, + { status: 403, env: {}, says: /answered 403 Forbidden .*rate limit for requests without a token; set GH_TOKEN/ }, + { status: 403, env: { GH_TOKEN: "narrow" }, says: /answered 403 Forbidden .*the token in GH_TOKEN or GITHUB_TOKEN can't read this repository's issues/ }, + { status: 404, env: {}, says: /answered 404 Not Found .*no repository owner\/repo is visible without a token; check package\.json's repository field, or set GH_TOKEN/ }, + { status: 404, env: { GH_TOKEN: "narrow" }, says: /answered 404 Not Found .*no repository owner\/repo is visible to the token in GH_TOKEN or GITHUB_TOKEN/ }, + { status: 418, env: {}, says: /answered 418 I'm a Teapot .*this check doesn't know that answer/ }, + ]; + for (const { status, env, says } of cases) { + const gh = await github(status, { message: "no" }); + try { + const result = await run(project(PKG), { GITHUB_API_URL: gh.url, ...env }); + assert.equal(result.code, 1, `${status}`); + assert.match(result.stderr, says); + } finally { + await gh.close(); + } + } +}); + +test("an answer that isn't a list stops the release", async () => { + const gh = await github(200, { message: "not a list" }); try { const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); assert.equal(result.code, 1); - assert.match(result.stderr, /answered 404 Not Found .*no repository owner\/repo is visible to this request; check package\.json's repository field/); + assert.match(result.stderr, /^release-holds: GitHub's answer wasn't a list of issues\./); } finally { await gh.close(); } }); test("GitHub out of reach stops the release, naming why", async () => { - // A port that was just free: nothing listens there now. (fetch refuses - // the discard port and others as "bad port" without connecting.) + // A port that was just free, so nothing listens there. Not the discard + // port: fetch refuses it, and others, as a "bad port" without connecting. const gh = await github(200, []); await gh.close(); const result = await run(project(PKG), { GITHUB_API_URL: gh.url }); @@ -192,8 +285,102 @@ test("GitHub out of reach stops the release, naming why", async () => { assert.match(result.stderr, /^release-holds: couldn't reach GitHub to read the release holds \(E[A-Z]+\)\./); }); +test("an API URL with a trailing slash asks the same path", async () => { + const gh = await github(200, []); + try { + const result = await run(project(PKG), { GITHUB_API_URL: `${gh.url}/` }); + assert.equal(result.code, 0, result.stderr); + assert.match(gh.seen[0]!.url, /^\/repos\/owner\/repo\/issues\?/); + } finally { + await gh.close(); + } +}); + +test("the stop message says how to go on where it ran", async () => { + const gh = await github(200, [HOLD]); + try { + const workflow = await run(project(PKG), { GITHUB_API_URL: gh.url, GITHUB_ACTIONS: "true" }); + assert.match(workflow.stderr, /then re-run this workflow for the tag\./); + const version = await run(project(PKG), { GITHUB_API_URL: gh.url, npm_lifecycle_event: "preversion" }); + assert.match(version.stderr, /then run npm version again\./); + const publish = await run(project(PKG), { GITHUB_API_URL: gh.url, npm_lifecycle_event: "prepublishOnly" }); + assert.match(publish.stderr, /then run npm publish again\./); + } finally { + await gh.close(); + } +}); + test("no repository to ask stops the release", async () => { const result = await run(project({ name: "fixture" }), { GITHUB_API_URL: "http://127.0.0.1:1" }); assert.equal(result.code, 1); - assert.match(result.stderr, /names no GitHub repository/); + assert.match(result.stderr, /names no repository, and GITHUB_REPOSITORY is unset/); +}); + +/** A throwaway package whose preversion and prepublishOnly are this + * repository's own, with the script beside it and a stand-in build. */ +function wiredProject(): string { + const scripts = (JSON.parse(readFileSync(PACKAGE_JSON, "utf8")) as { scripts: Record }).scripts; + const dir = project({ + ...PKG, + scripts: { preversion: scripts.preversion, prepublishOnly: scripts.prepublishOnly, build: "node -e 0" }, + }); + mkdirSync(join(dir, "scripts")); + copyFileSync(SCRIPT, join(dir, "scripts", "release-holds.mjs")); + writeFileSync(join(dir, ".npmrc"), ""); + return dir; +} + +/** npm, kept to the throwaway directory: its own cache and user config, + * and the stand-in for its registry and GitHub's API. */ +function npm(dir: string, args: string[], api: string) { + return exec("npm", args, dir, { + HOME: dir, + npm_config_cache: join(dir, ".npm-cache"), + npm_config_userconfig: join(dir, ".npmrc"), + npm_config_registry: api, + npm_config_update_notifier: "false", + npm_config_fund: "false", + npm_config_audit: "false", + GITHUB_API_URL: api, + }); +} + +test("this repository's preversion stops npm version before the bump while a hold is open", async () => { + const gh = await github(200, [HOLD]); + try { + const dir = wiredProject(); + const result = await npm(dir, ["version", "patch", "--no-git-tag-version"], gh.url); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /#300 DefaultErrorPolicy/); + assert.match(result.stderr, /then run npm version again\./); + assert.equal(JSON.parse(readFileSync(join(dir, "package.json"), "utf8")).version, "1.0.0"); + } finally { + await gh.close(); + } +}); + +test("with no hold open, this repository's preversion lets npm version bump", async () => { + const gh = await github(200, []); + try { + const dir = wiredProject(); + const result = await npm(dir, ["version", "patch", "--no-git-tag-version"], gh.url); + assert.equal(result.code, 0, result.stderr); + assert.equal(JSON.parse(readFileSync(join(dir, "package.json"), "utf8")).version, "1.0.1"); + assert.ok(gh.seen.some((s) => s.url.startsWith("/repos/owner/repo/issues?"))); + } finally { + await gh.close(); + } +}); + +test("this repository's prepublishOnly stops npm publish while a hold is open", async () => { + const gh = await github(200, [HOLD]); + try { + const dir = wiredProject(); + const result = await npm(dir, ["publish", "--dry-run"], gh.url); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /#300 DefaultErrorPolicy/); + assert.match(result.stderr, /then run npm publish again\./); + } finally { + await gh.close(); + } }); From cb2a336589a91e41b94a4b326225a5cdbbc1688b Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Wed, 14 Oct 2026 04:50:42 -0400 Subject: [PATCH 5/5] build(release): a 404 names the setting that named the repository When package.json names no repository, the hold check asks the one GITHUB_REPOSITORY names. A 404 for it then said to check package.json's repository field, which named nothing. The hint now names whichever setting named the repository: package.json's repository field, or GITHUB_REPOSITORY. A test runs both: package.json naming a repository (with GITHUB_REPOSITORY set to another, which the check ignores), and package.json naming none. Under the old wording the second case fails. --- scripts/release-holds.mjs | 15 +++++++++------ test/release-holds.test.ts | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/scripts/release-holds.mjs b/scripts/release-holds.mjs index dcbae89e9..7a4fd2530 100644 --- a/scripts/release-holds.mjs +++ b/scripts/release-holds.mjs @@ -36,24 +36,27 @@ function githubRepository(field) { return { unreadable: value }; } +/** The repository to ask, and where it was named, for a message that has to + * send a maintainer to the setting that named it. */ function repository() { + const fromEnvironment = { repo: process.env.GITHUB_REPOSITORY, namedBy: "GITHUB_REPOSITORY" }; let pkg; try { pkg = JSON.parse(readFileSync("package.json", "utf8")); } catch (error) { - if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; + if (fromEnvironment.repo) return fromEnvironment; fail(`release-holds: can't read package.json (${error.message}), and GITHUB_REPOSITORY is unset`); } const named = githubRepository(pkg.repository); - if (named.repo) return named.repo; + if (named.repo) return { repo: named.repo, namedBy: "package.json's repository field" }; if (named.unreadable) { fail(`release-holds: package.json's repository field (${named.unreadable}) names no GitHub repository this check can ask`); } - if (process.env.GITHUB_REPOSITORY) return process.env.GITHUB_REPOSITORY; + if (fromEnvironment.repo) return fromEnvironment; fail("release-holds: package.json names no repository, and GITHUB_REPOSITORY is unset"); } -const repo = repository(); +const { repo, namedBy } = repository(); const api = (process.env.GITHUB_API_URL ?? "https://api.github.com").replace(/\/+$/, ""); const token = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; const where = `https://github.com/${repo}/issues?q=is%3Aissue+is%3Aopen+label%3A${LABEL}`; @@ -70,8 +73,8 @@ function refusal(status) { } if (status === 404) { return token - ? `no repository ${repo} is visible to the token in GH_TOKEN or GITHUB_TOKEN; check package.json's repository field and the token's access` - : `no repository ${repo} is visible without a token; check package.json's repository field, or set GH_TOKEN if the repository is private`; + ? `no repository ${repo} is visible to the token in GH_TOKEN or GITHUB_TOKEN; check ${namedBy} and the token's access` + : `no repository ${repo} is visible without a token; check ${namedBy}, or set GH_TOKEN if the repository is private`; } if (status >= 500) return "GitHub had an error; run this again once it answers normally"; return "this check doesn't know that answer; see what GitHub shows"; diff --git a/test/release-holds.test.ts b/test/release-holds.test.ts index 3ea060352..04bdf428c 100644 --- a/test/release-holds.test.ts +++ b/test/release-holds.test.ts @@ -226,6 +226,23 @@ test("GITHUB_REPOSITORY names the repository when package.json names none", asyn } }); +test("a repository GitHub doesn't know sends the maintainer to the setting that named it", async () => { + const cases: Array<{ pkg: Record; says: RegExp }> = [ + { pkg: PKG, says: /no repository owner\/repo is visible without a token; check package\.json's repository field, or set GH_TOKEN/ }, + { pkg: { name: "fixture" }, says: /no repository gone\/place is visible without a token; check GITHUB_REPOSITORY, or set GH_TOKEN/ }, + ]; + for (const { pkg, says } of cases) { + const gh = await github(404, { message: "Not Found" }); + try { + const result = await run(project(pkg), { GITHUB_API_URL: gh.url, GITHUB_REPOSITORY: "gone/place" }); + assert.equal(result.code, 1); + assert.match(result.stderr, says); + } finally { + await gh.close(); + } + } +}); + test("a package.json that isn't JSON stops the release, saying so", async () => { const result = await run(project("{ not json"), { GITHUB_API_URL: "http://127.0.0.1:1" }); assert.equal(result.code, 1);