From fce54f6beaf75e8f99db20046bef6604c4f95461 Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Tue, 6 Oct 2026 11:09:59 +0200 Subject: [PATCH 1/2] ci(release): ship v1.x versions through a release PR v1.x now requires pull requests, so the land job's direct ref update to the release line fails with a 422 after the packages are already staged. The workflow gains a mode input. mode=release-pr derives the bump in a job with no write credential and passes it as a two-file patch to a job that installs nothing, which commits it via the release App and opens a chore(release) PR into the dispatch branch. mode=publish builds the newest first-parent commit that changed the package.json version, so it only ships what a merged release PR introduced, then tags, releases and stages as before. Nothing in the workflow writes to the release line. --- .github/workflows/publish-npm.yml | 260 ++++++++++++++-------------- CLAUDE.md | 21 ++- scripts/release/bump.mts | 93 ++-------- scripts/release/github-api.mts | 77 +++++++- scripts/release/mint-app-token.mjs | 15 +- scripts/release/open-release-pr.mts | 147 ++++++++++++++++ scripts/release/promote.mts | 93 ---------- scripts/release/release-branch.mts | 89 ++++------ test/release-open-pr.test.mts | 96 ++++++++++ test/release-promote.test.mts | 79 --------- test/release-workflow.test.mts | 75 ++++++-- test/script-run-main.test.mts | 2 +- 12 files changed, 571 insertions(+), 476 deletions(-) create mode 100644 scripts/release/open-release-pr.mts delete mode 100644 scripts/release/promote.mts create mode 100644 test/release-open-pr.test.mts delete mode 100644 test/release-promote.test.mts diff --git a/.github/workflows/publish-npm.yml b/.github/workflows/publish-npm.yml index 9f1d3094b..e49709a38 100644 --- a/.github/workflows/publish-npm.yml +++ b/.github/workflows/publish-npm.yml @@ -6,29 +6,23 @@ name: Publish to npm registry # 1. Between releases nobody touches the version. package.json keeps the # last released number and user-facing notes accrue under the # CHANGELOG's `## [Unreleased]` section as they land. -# 2. Dispatch this workflow with dry-run=true (the default): the `verify` -# job derives and PRINTS the version it would ship, then builds, packs, -# and smoke-tests all three variants. It uploads a GitHub artifact and -# verifies its download. It creates no npm stages, tags, releases, or -# release branches, and needs no publish credential. -# 3. Dispatch with dry-run=false + dist-tag=latest. This branch is the line -# customers consume, so it owns `latest`; the guard below refuses that -# tag from anywhere else, and the default branch (the 2.x prerelease -# line) publishes under next/beta/canary/rc. -# 4. `verify` now BUMPS in-run: scripts/release/bump.mts derives the next -# version from the commits since the last release, writes package.json + -# CHANGELOG.md, and commits the pair via the release App onto a throwaway -# `npm-publish-v` branch. No hand `chore(release):` commit ever -# lands on v1.x, and the release line is not touched until the run is -# proven. -# 5. `publish` cuts the v tag + the immutable GitHub release — both -# belong to the `socket` package, exactly one of each per run — then -# STAGES those exact tarballs. +# 2. Dispatch with mode=release-pr. The `derive` job runs +# scripts/release/bump.mts to pick the next version and rewrite +# package.json + CHANGELOG.md. With dry-run=true (the default) it only +# prints the version. With dry-run=false the `release-pr` job commits the +# pair via the release App onto `npm-publish-v` and opens a +# `chore(release): X.Y.Z` PR into this branch. +# 3. A human reviews and squash-merges the release PR. v1.x is protected, so +# that PR is the only way a version bump reaches it. +# 4. Dispatch with mode=publish. A dry run builds, packs, and smoke-tests +# HEAD and creates nothing. With dry-run=false the `verify` job checks out +# the newest commit that changed package.json's version (the merged +# release PR) and builds that. This branch is the line customers consume, +# so it owns `latest`. The guard below refuses that tag from anywhere else. +# 5. `publish` cuts the v tag + the immutable GitHub release at that +# commit, then STAGES the exact tarballs `verify` packed. # 6. A human promotes each staged upload (`pnpm stage approve` with web -# 2FA, or the npm web UI); nothing is public until then. -# 7. `land` fast-forwards v1.x to the bump commit once staging succeeded, -# and DELETES the throwaway branch when anything failed — so a failed run -# leaves the release line exactly as it found it. +# 2FA, or the npm web UI). Nothing is public until then. # # Stable tags from this major version reserve their versions, including tags # on unlanded bump commits. Reachable tags anchor the changelog history. @@ -38,13 +32,11 @@ name: Publish to npm registry # makes it minor. A MAJOR is never derived — a breaking commit stops the bump # and asks a human to pass release-as. # -# THREE JOBS, ONE CREDENTIAL BOUNDARY. `verify` binds no environment and mints -# no OIDC token, so nothing it runs — install scripts, build tooling, actions — -# can reach a publish credential. The release App token it does hold is a -# contents:write GIT credential, never a registry one, so the registry boundary -# is untouched. `publish` holds the registry credential and does almost nothing: -# no checkout, no install, no build. It publishes the exact bytes `verify` -# packed and proved, so what shipped is what was tested. +# CREDENTIAL BOUNDARIES. `derive` and `verify` install and build, and hold no +# write credential of any kind. `release-pr` holds the release App token and +# runs no installed code. `publish` holds the registry credential and does +# almost nothing: no checkout, no install, no build. It publishes the exact +# bytes `verify` packed and proved, so what shipped is what was tested. on: workflow_dispatch: @@ -54,18 +46,21 @@ on: required: false default: 'latest' type: string - dry-run: - description: 'Build everything but do NOT publish, tag, or cut a release. Defaults to true so an accidental dispatch never reaches the registry — set to false for a real release.' + mode: + description: 'release-pr opens the version bump PR. publish releases the newest merged release PR.' required: false - default: true - type: boolean - bump: - description: 'Derive the version + CHANGELOG in-run and commit them via the release App. Leave on. Turning it off publishes whatever version the tree already carries, which is only ever right for re-running a run whose bump commit already landed.' + default: 'publish' + type: choice + options: + - publish + - release-pr + dry-run: + description: 'Defaults to true so an accidental dispatch writes nothing. release-pr: print the next version without opening a PR. publish: build everything but do NOT publish, tag, or cut a release.' required: false default: true type: boolean release-as: - description: 'Force the bump level instead of deriving it from the commits. MAJOR is never derived — a breaking change stops the bump until a human picks major here.' + description: 'release-pr only. Force the bump level instead of deriving it from the commits. MAJOR is never derived — a breaking change stops the bump until a human picks major here.' required: false default: '' type: choice @@ -95,6 +90,7 @@ jobs: # what ship, so what was tested is what publishes. verify: name: Verify and pack + if: ${{ inputs.mode == 'publish' }} runs-on: ubuntu-latest permissions: @@ -104,10 +100,7 @@ jobs: outputs: artifact-id: ${{ steps.upload.outputs.artifact-id }} artifact-digest: ${{ steps.upload.outputs.artifact-digest }} - # Empty on a dry run or a bump=false dispatch, which is what the `land` - # job keys off: no branch means there is nothing to land or discard. - release-branch: ${{ steps.bump.outputs.release-branch }} - sha: ${{ steps.release-meta.outputs.sha || steps.bump.outputs.sha }} + sha: ${{ steps.release-meta.outputs.sha }} version: ${{ steps.release-meta.outputs.version }} steps: @@ -137,9 +130,11 @@ jobs: fi echo "dist-tag 'latest' is allowed from $REF (consumable line: $LATEST_BRANCH)." - # Full history anchors the changelog. All tags expose reserved versions, - # including release tags on bump commits that did not land. - - name: Checkout source + # Full history locates the release commit here and anchors the changelog + # in `derive`. All tags expose reserved versions, including release tags + # on bump commits that did not land. + - &checkout-full-history + name: Checkout source shell: bash env: CHECKOUT_REF: ${{ github.sha }} @@ -158,7 +153,29 @@ jobs: git fetch --tags origin "$CHECKOUT_REF" '+refs/heads/*:refs/remotes/origin/*' git checkout -q --detach FETCH_HEAD - - name: Install pnpm + # A real publish ships the newest first-parent commit that changed + # package.json's version, i.e. the merged release PR. Commits merged after + # it wait for the next release, and a version nobody reviewed in a release + # PR never ships. A dry run packs HEAD as it is. + - name: Check out the release commit + if: ${{ inputs.dry-run == false }} + run: | + set -euo pipefail + for COMMIT in $(git rev-list --first-parent --max-count=500 HEAD -- package.json); do + VERSION=$(git show "$COMMIT:package.json" | jq -r .version) + PREVIOUS=$(git show "$COMMIT^:package.json" | jq -r .version) + if [ "$VERSION" != "$PREVIOUS" ]; then + git checkout -q --detach "$COMMIT" + echo "Releasing $VERSION from $(git log -1 --format='%h %s')." + exit 0 + fi + done + echo "::error::No commit on this branch changed the package.json version." >&2 + echo "::error::Fix: dispatch with mode=release-pr, merge that PR, then dispatch mode=publish." >&2 + exit 1 + + - &install-pnpm + name: Install pnpm shell: bash run: | # zizmor: ignore[github-env] # pnpm 11 is required for `pnpm stage publish` (the staged upload @@ -202,13 +219,15 @@ jobs: "${RUNNER_TEMP}/pnpm-bin/pnpm" stage --help > /dev/null echo "pnpm stage resolves via the pinned binary." - - name: Install Node.js + - &install-node + name: Install Node.js shell: bash env: NODE_VERSION: 25.9.0 run: node scripts/ci/setup-node.mjs --version "$NODE_VERSION" - - name: Download sfw + - &download-sfw + name: Download sfw shell: bash env: GH_TOKEN: ${{ github.token }} @@ -274,7 +293,8 @@ jobs: echo "SOCKET_API_KEY=$SOCKET_API_KEY" >> "${GITHUB_ENV:-/dev/null}" fi - - name: Create sfw shims + - &create-sfw-shims + name: Create sfw shims shell: bash run: | # zizmor: ignore[github-env] SHIM_DIR="${RUNNER_TEMP:-/tmp}/sfw-shim" @@ -328,50 +348,13 @@ jobs: echo "$SHIM_DIR" >> "${GITHUB_PATH:-/dev/null}" echo "SFW_SHIM_DIR=$SHIM_DIR" >> "${GITHUB_ENV:-/dev/null}" - - name: Install dependencies + - &install-dependencies + name: Install dependencies run: pnpm install --loglevel error - # The release App signs the bump commit through the GitHub API, so the - # workflow's own GITHUB_TOKEN stays contents: read for the whole run. The - # mint is contents:write and nothing more — it moves branch refs, it does - # not touch the registry. A dry run and a bump=false dispatch both skip it, - # since neither writes a commit. - - name: Mint release App token - if: ${{ inputs.dry-run == false && inputs.bump }} - id: release-app - env: - APP_PRIVATE_KEY: ${{ secrets.SOCKET_RELEASE_APP_PRIVATE_KEY }} - CLIENT_ID: ${{ vars.SOCKET_RELEASE_CLIENT_ID }} - OWNER: ${{ github.repository_owner }} - PERMISSIONS: '{"contents":"write"}' - REPOSITORIES: ${{ github.event.repository.name }} - run: node scripts/release/mint-app-token.mjs - - # Derive the version from the commits since the last release, write - # package.json + CHANGELOG.md, and commit the pair via the release App onto - # a throwaway npm-publish-v branch — NOT v1.x. The checkout resets - # to that commit, so everything built and packed below comes from the exact - # commit the release will be tagged at. The `land` job fast-forwards v1.x to - # it only once staging succeeded. - # - # In a dry run, this step prints the next version without writing a bump - # or opening a branch. The following steps pack the tree's current version, - # upload a GitHub artifact, and verify its download. - - name: Bump version and changelog - if: ${{ inputs.bump }} - id: bump - env: - RELEASE_APP_TOKEN: ${{ steps.release-app.outputs.token }} - RELEASE_AS: ${{ inputs.release-as }} - run: | - node scripts/release/bump.mts \ - ${RELEASE_AS:+--release-as "$RELEASE_AS"} \ - ${{ inputs.dry-run && '--dry-run' || '' }} - - # Whatever produced the version — the bump above, or the tree itself on a - # bump=false dispatch — it must be a bare X.Y.Z before anything reaches the - # registry. A prerelease-suffixed version is a work-in-progress marker, not - # a releasable one, so fail closed rather than publish it. + # The version must be a bare X.Y.Z before anything reaches the registry. A + # prerelease-suffixed version is a work-in-progress marker, not a + # releasable one, so fail closed rather than publish it. - name: Refuse a publish on a non-release version if: ${{ inputs.dry-run == false }} run: | @@ -379,7 +362,7 @@ jobs: case "$VERSION" in *-*) echo "::error::package.json version is '$VERSION' — a prerelease version, not a releasable one." >&2 - echo "::error::Where: the tree this run will pack, after the bump stage." >&2 + echo "::error::Where: the tree this run will pack." >&2 echo "::error::Saw: a prerelease suffix; wanted a bare X.Y.Z." >&2 echo "::error::Fix: land a release-shaped version on v1.x, then re-dispatch." >&2 exit 1 @@ -459,11 +442,6 @@ jobs: # the commit are read once here and handed to the publish job. The # publish job never checks the repo out — it only needs these two # strings plus the tarballs. - # - # The SHA comes from HEAD, not github.sha: the bump reset the checkout to - # the commit it created on the throwaway branch, and github.sha still names - # the pre-bump dispatch commit. Tagging that one would mark a commit whose - # package.json carries the PREVIOUS version. - name: Resolve release metadata id: release-meta run: | @@ -715,35 +693,59 @@ jobs: done echo "All three packages staged. Promote each with: pnpm stage approve " - # Decide what happens to the bump. This is the ONLY job that writes to v1.x, - # and it runs after everything that could fail has either succeeded or not. - # - # The run staged all three packages -> fast-forward v1.x to the bump commit, - # then delete the throwaway branch. - # Anything else -> delete the throwaway branch and leave v1.x untouched, so - # a failed run costs nothing but the burned version number. - # - # The fast-forward preserves the App's exact signed SHA — the SHA the release - # tag already points at — which a merge or squash would rewrite. It is a ref - # PATCH, not a pull request: a fresh bump branch has no protected-branch rules - # to satisfy, so a PR route would park the release behind checks it can never - # pass, and there is nothing to review in a machine-generated bump anyway. - # - # `always()` is what makes the discard leg reachable — this has to run when - # `publish` failed, not only when it succeeded. The release-branch guard keeps - # it a no-op for dry runs and bump=false dispatches, which never open a branch. - land: - name: Land the bump - needs: [verify, publish] - if: ${{ always() && needs.verify.outputs.release-branch != '' }} + # Derive the next version and write package.json + CHANGELOG.md. Runs the + # installed toolchain, so it holds no write credential. The bump leaves this + # job as a patch limited to those two files. + derive: + name: Derive the release bump + if: ${{ inputs.mode == 'release-pr' }} + runs-on: ubuntu-latest + + permissions: + contents: read + + outputs: + patch: ${{ steps.patch.outputs.patch }} + version: ${{ steps.bump.outputs.version }} + + steps: + - *checkout-full-history + - *install-pnpm + - *install-node + - *download-sfw + - *create-sfw-shims + - *install-dependencies + + - name: Bump version and changelog + id: bump + env: + RELEASE_AS: ${{ inputs.release-as }} + run: | + node scripts/release/bump.mts \ + ${RELEASE_AS:+--release-as "$RELEASE_AS"} \ + ${{ inputs.dry-run && '--dry-run' || '' }} + + - name: Export the bump as a patch + if: ${{ inputs.dry-run == false }} + id: patch + run: | + echo "patch=$(git diff --binary HEAD -- CHANGELOG.md package.json | base64 -w0)" >> "$GITHUB_OUTPUT" + git diff --stat HEAD + + # Commit the bump via the release App and open the release PR. This job + # installs nothing, so no third-party code shares a job with the App token. + release-pr: + name: Open the release PR + needs: derive + if: ${{ inputs.mode == 'release-pr' && inputs.dry-run == false }} runs-on: ubuntu-latest permissions: contents: read steps: - # promote.mts is the only thing needed from the tree, and it is identical - # on the dispatch ref and the bump commit, so the plain checkout is enough. + # The bump commit's parent is github.sha, so a shallow checkout of it is + # enough to apply the patch and read the base tree. - name: Checkout source shell: bash env: @@ -763,11 +765,12 @@ jobs: git fetch --no-tags --depth=1 origin "$CHECKOUT_REF" git checkout -q --detach FETCH_HEAD - - name: Install Node.js - shell: bash + - *install-node + + - name: Apply the bump env: - NODE_VERSION: 25.9.0 - run: node scripts/ci/setup-node.mjs --version "$NODE_VERSION" + PATCH_B64: ${{ needs.derive.outputs.patch }} + run: printf '%s' "$PATCH_B64" | base64 -d | git apply - name: Mint release App token id: release-app @@ -775,17 +778,12 @@ jobs: APP_PRIVATE_KEY: ${{ secrets.SOCKET_RELEASE_APP_PRIVATE_KEY }} CLIENT_ID: ${{ vars.SOCKET_RELEASE_CLIENT_ID }} OWNER: ${{ github.repository_owner }} - PERMISSIONS: '{"contents":"write"}' + PERMISSIONS: '{"contents":"write","pull_requests":"write"}' REPOSITORIES: ${{ github.event.repository.name }} run: node scripts/release/mint-app-token.mjs - - name: Land or discard the bump + - name: Commit the bump and open the release PR env: - BRANCH: ${{ needs.verify.outputs.release-branch }} RELEASE_APP_TOKEN: ${{ steps.release-app.outputs.token }} - SHA: ${{ needs.verify.outputs.sha }} - run: | - node scripts/release/promote.mts \ - --branch "$BRANCH" \ - --sha "$SHA" \ - ${{ needs.publish.result != 'success' && '--discard' || '' }} + VERSION: ${{ needs.derive.outputs.version }} + run: node scripts/release/open-release-pr.mts --version "$VERSION" diff --git a/CLAUDE.md b/CLAUDE.md index 5b96979d9..99320a5cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,17 +159,22 @@ Validate all three bindings after a workflow or environment rename. the work lands. The release promotes that block verbatim under the new version heading. If nothing accrued, the release falls back to a section derived from the Conventional Commits in range. -- Dispatch the workflow with `dry-run: true` (the default) to see which - version it would ship. It uploads a GitHub artifact and verifies its download. - It creates no npm stages, tags, releases, or release branches. -- Dispatch with `dry-run: false` to release. `scripts/release/bump.mts` picks - the version, writes `package.json` + `CHANGELOG.md`, and commits them via - the release App onto a throwaway `npm-publish-v` branch. `v1.x` is - fast-forwarded to that commit only after all three packages are staged. +- Dispatch with `mode: release-pr` and `dry-run: true` (the default) to see + which version the next release would get. Nothing is written. +- Dispatch with `mode: release-pr` and `dry-run: false` to open the release + PR. `scripts/release/bump.mts` picks the version and writes `package.json` + + `CHANGELOG.md`, and `scripts/release/open-release-pr.mts` commits them via + the release App onto `npm-publish-v` and opens a + `chore(release): X.Y.Z` PR into `v1.x`. Review it and squash-merge it. +- Dispatch with `mode: publish` and `dry-run: false` to release. It builds the + newest commit that changed the `package.json` version, tags it, cuts the + GitHub release, and stages all three packages for `pnpm stage approve`. + `mode: publish` with `dry-run: true` packs and smoke-tests HEAD and creates + nothing. - The level is patch by default and minor when a `feat:` is in range. A major is never derived — a breaking commit stops the bump until someone passes `release-as: major`. -- A tag reserves its version even when staging or landing fails. Stable tags +- A tag reserves its version even when staging fails. Stable tags from this major version set the reservation floor. Reachable tags anchor the changelog history. The next release skips all reserved versions. - Run `pnpm run release:preflight --version ` to check all three diff --git a/scripts/release/bump.mts b/scripts/release/bump.mts index 42efd0801..b6fd7f858 100644 --- a/scripts/release/bump.mts +++ b/scripts/release/bump.mts @@ -1,24 +1,18 @@ #!/usr/bin/env node /** * @file The CI bump stage. Derives the next version from the commits landed - * since the last release, writes package.json + CHANGELOG.md, and commits the - * pair via the release App onto a throwaway `npm-publish-v` branch. - * - * Nothing here is hand-run. The publish-npm workflow calls it between install - * and build, so the tarballs it packs carry the derived version and the commit - * they claim to be built from. `promote.mts` lands or deletes the branch once - * the run is decided. + * since the last release and writes package.json + CHANGELOG.md into the + * working tree. `open-release-pr.mts` commits the pair and opens the release + * PR from a separate job that never installs dependencies. * * Usage: * node scripts/release/bump.mts [--dry-run] [--release-as major|minor|patch] */ -import { execFile as execFileCallback } from 'node:child_process' import { appendFileSync, readFileSync, writeFileSync } from 'node:fs' import path from 'node:path' import process from 'node:process' import { fileURLToPath } from 'node:url' -import { promisify } from 'node:util' import { changelogHeading, @@ -26,23 +20,14 @@ import { promoteChangelog, repoBaseUrl, } from './changelog.mts' -import { commitViaGithubApi } from './github-api.mts' import { readReleaseCommits, readReleaseHistory } from './history.mts' import { readPublishedVersion } from './registry.mts' -import { - discardReleaseBranch, - openReleaseBranch, - resolveReleaseEnv, -} from './release-branch.mts' import { deriveNextVersion, parseConventionalCommits } from './version.mts' import { isMainModule } from '../lib/is-main-module.mts' import { runMain } from '../lib/run-main.mts' -import type { ReleaseBranch } from './release-branch.mts' import type { ScriptMeta } from '../lib/run-main.mts' -const execFile = promisify(execFileCallback) - const rootPath = path.join( path.dirname(fileURLToPath(import.meta.url)), '..', @@ -61,14 +46,6 @@ function log(message: string): void { process.stdout.write(`[bump] ${message}\n`) } -async function git(args: readonly string[]): Promise { - const { stdout } = await execFile('git', [...args], { - cwd: rootPath, - maxBuffer: 64 * 1024 * 1024, - }) - return stdout -} - function readPackageJson(): { parsed: PackageJsonShape; raw: string } { const raw = readFileSync(path.join(rootPath, 'package.json'), 'utf8') return { parsed: JSON.parse(raw) as PackageJsonShape, raw } @@ -179,77 +156,27 @@ async function main(): Promise { return } - const env = resolveReleaseEnv() writeFileSync( path.join(rootPath, 'package.json'), writeManifestVersion(manifest.raw, derived.version), ) writeFileSync(changelogPath, promoted.changelog) - - const parentSha = (await git(['rev-parse', 'HEAD'])).trim() - const baseTreeSha = (await git(['rev-parse', 'HEAD^{tree}'])).trim() - const files = ['CHANGELOG.md', 'package.json'].map(relPath => ({ - content: readFileSync(path.join(rootPath, relPath), 'utf8'), - path: relPath, - })) - const releaseBranch: ReleaseBranch = await openReleaseBranch({ - env, - parentSha, - version: derived.version, - }) - // Past this point any failure must nuke the branch, otherwise a leftover - // npm-publish-v accumulates. The release line is never touched here, - // so the no-version-creep invariant holds either way. - try { - const sha = await commitViaGithubApi({ - baseTreeSha, - branch: releaseBranch.branch, - files, - message: `chore(release): ${derived.version}`, - parentSha, - repo: env.repo, - token: env.token, - }) - // The checkout runs with persist-credentials off, so the fetch carries the - // App token inline rather than writing it into .git/config. - const auth = Buffer.from(`x-access-token:${env.token}`).toString('base64') - await git([ - '-c', - `http.https://github.com/.extraheader=AUTHORIZATION: basic ${auth}`, - 'fetch', - '--no-tags', - 'origin', - `refs/heads/${releaseBranch.branch}`, - ]) - await git(['reset', '--hard', sha]) - log( - `${derived.version} committed ${sha.slice(0, 7)} on ${releaseBranch.branch} ` + - 'via the release App.', - ) - emitOutputs({ - 'release-branch': releaseBranch.branch, - sha, - version: derived.version, - }) - } catch (e) { - await discardReleaseBranch(releaseBranch) - throw e - } + log(`wrote ${derived.version} to package.json and CHANGELOG.md.`) + emitOutputs({ version: derived.version }) } const SCRIPT_META: ScriptMeta = { describe: - 'derives the next release version from the landed commits and commits package.json + CHANGELOG.md via the release App', + 'derives the next release version from the landed commits and writes package.json + CHANGELOG.md', help: `Usage: node scripts/release/bump.mts [flags] - --dry-run derive and print the version without opening - a release branch or committing anything + --dry-run derive and print the version without writing + package.json or CHANGELOG.md --release-as major|minor|patch force the bump level instead of deriving it from the conventional commits - The publish-npm workflow runs this between install and build. It is not a - hand-run script: it needs RELEASE_APP_TOKEN and the GitHub Actions - environment to reach the release App.`, + The publish-npm workflow runs this in its release-pr mode. It only edits + the working tree, so a local run is safe to inspect and discard.`, } if (isMainModule(import.meta.url)) { diff --git a/scripts/release/github-api.mts b/scripts/release/github-api.mts index a8934d2f5..f480bb5c5 100644 --- a/scripts/release/github-api.mts +++ b/scripts/release/github-api.mts @@ -1,6 +1,7 @@ /** - * @file The GitHub REST calls the release flow needs: branch refs and a signed - * commit built out of git objects (blob → tree → commit → ref). + * @file The GitHub REST calls the release flow needs: branch refs, a signed + * commit built out of git objects (blob → tree → commit → ref), and the + * release pull request. * * A commit created through the API is web-flow VERIFIED without a local GPG or * SSH key, which is the only way CI can land a commit on a branch that @@ -27,7 +28,7 @@ export interface GithubRequestConfig { readonly method: string // Path below the API origin, e.g. `/repos/owner/name/git/refs`. readonly path: string - // Token with contents:write — the release App installation token in CI. + // The release App installation token in CI. readonly token: string } @@ -97,10 +98,8 @@ export async function createBranchRef( } /** - * Advance `refs/heads/` to `sha`. With `force` false — the default — - * GitHub rejects a non-fast-forward advance with 422, which is what keeps the - * post-publish landing honest: if the release line moved to a commit this one - * does not descend from, the run stops loudly instead of rewriting work. + * Advance `refs/heads/` to `sha`. With `force` false, the default, + * GitHub rejects a non-fast-forward advance with 422. */ export async function updateBranchRef( config: WriteBranchRefConfig, @@ -220,3 +219,67 @@ export async function commitViaGithubApi( }) return commit!.sha } + +export interface PullRequest { + readonly html_url: string + readonly number: number +} + +export interface ReleasePullRequestConfig { + readonly apiUrl?: string | undefined + // Branch the PR merges into. + readonly base: string + readonly body: string + // Branch the PR merges from, in the same repository. + readonly head: string + readonly repo: string + readonly title: string + readonly token: string +} + +/** + * Open a PR from `head` into `base`, or return the open one that already + * exists for that pair. A re-run force-resets `head`, so the existing PR picks + * up the new commit by itself and only its title and body need refreshing. + */ +export async function upsertPullRequest( + config: ReleasePullRequestConfig, +): Promise { + const cfg = { __proto__: null, ...config } as ReleasePullRequestConfig + const owner = cfg.repo.split('/')[0] + const query = new URLSearchParams({ + base: cfg.base, + head: `${owner}:${cfg.head}`, + state: 'open', + }) + const existing = await githubRequest({ + apiUrl: cfg.apiUrl, + method: 'GET', + path: `/repos/${cfg.repo}/pulls?${query}`, + token: cfg.token, + }) + const open = existing?.[0] + if (open) { + const updated = await githubRequest({ + apiUrl: cfg.apiUrl, + body: { body: cfg.body, title: cfg.title }, + method: 'PATCH', + path: `/repos/${cfg.repo}/pulls/${open.number}`, + token: cfg.token, + }) + return updated! + } + const created = await githubRequest({ + apiUrl: cfg.apiUrl, + body: { + base: cfg.base, + body: cfg.body, + head: cfg.head, + title: cfg.title, + }, + method: 'POST', + path: `/repos/${cfg.repo}/pulls`, + token: cfg.token, + }) + return created! +} diff --git a/scripts/release/mint-app-token.mjs b/scripts/release/mint-app-token.mjs index fc9cac1bd..b88e864a0 100644 --- a/scripts/release/mint-app-token.mjs +++ b/scripts/release/mint-app-token.mjs @@ -6,8 +6,8 @@ * installation token scoped by the PERMISSIONS env. The token is masked, then * handed back via $GITHUB_OUTPUT. * - * The publish-npm workflow runs this to get the contents:write token that - * signs the bump commit and lands it — the workflow's own GITHUB_TOKEN stays + * The publish-npm workflow runs this to get the token that signs the bump + * commit and opens the release PR — the workflow's own GITHUB_TOKEN stays * contents:read. PERMISSIONS is always passed non-blank so the mint is * least-privilege; an empty object would mint blanket permissions and is * rejected below. @@ -135,8 +135,8 @@ export function formatAppPermissionLabel(scope) { // The scopes the REQUEST asks for that the installation's own grant does not // cover, each with what was wanted vs what is actually granted. This is the // PREFLIGHT: an installation missing a scope 422s the mint (or, worse, a widened -// request lands and the permission is only exercised LATER — a promote PR 403ing -// after the irreversible publish). Comparing the grant up front turns that into +// request lands and the permission is only exercised LATER — the release PR +// 403ing after its branch is written). Comparing the grant up front turns that into // a refusal before anything is published. Pure + exported so it is // unit-testable. export function findMissingAppPermissions(config) { @@ -175,7 +175,7 @@ export function formatAppPermissionShortfall(config) { } lines.push( ` A missing scope fails LATE otherwise — the mint 422s, or the permission is first`, - ` exercised after the irreversible publish (the promote PR 403s mid-release).`, + ` exercised after the release branch is written (the release PR 403s).`, ` Fix: ${url}`, ) for (const entry of missing) { @@ -245,10 +245,7 @@ async function main() { } // PREFLIGHT: the installation's own grant must already cover every requested - // scope. Runs before the mint and therefore before any publish/promote — the - // widened `pull_requests: write` request is only exercised by the promote PR - // that follows a successful publish, so without this the shortfall surfaces - // as a 403 in the irreversible window. + // scope, so a shortfall refuses before any branch or commit is written. if (permissions !== undefined) { const missing = findMissingAppPermissions({ granted: installation.permissions, diff --git a/scripts/release/open-release-pr.mts b/scripts/release/open-release-pr.mts new file mode 100644 index 000000000..944db98b0 --- /dev/null +++ b/scripts/release/open-release-pr.mts @@ -0,0 +1,147 @@ +#!/usr/bin/env node +/** + * @file Commit the bump that `bump.mts` wrote into the working tree via the + * release App and open the release PR into the release line. The + * publish-npm workflow runs this in a job with no dependency install, so the + * App token never shares a job with third-party code. + * + * Usage: + * node scripts/release/open-release-pr.mts --version 1.5.1 + */ + +import { execFile as execFileCallback } from 'node:child_process' +import { appendFileSync, readFileSync } from 'node:fs' +import path from 'node:path' +import process from 'node:process' +import { fileURLToPath } from 'node:url' +import { promisify } from 'node:util' + +import { commitViaGithubApi } from './github-api.mts' +import { + discardReleaseBranch, + openReleaseBranch, + openReleasePullRequest, + resolveReleaseEnv, +} from './release-branch.mts' +import { isMainModule } from '../lib/is-main-module.mts' +import { runMain } from '../lib/run-main.mts' + +import type { ScriptMeta } from '../lib/run-main.mts' + +const execFile = promisify(execFileCallback) + +const rootPath = path.join( + path.dirname(fileURLToPath(import.meta.url)), + '..', + '..', +) + +// The only files a bump may change. Anything else in the diff means the +// derivation job wrote something it should not have. +const BUMP_FILES = ['CHANGELOG.md', 'package.json'] + +function readFlag(argv: readonly string[], name: string): string | undefined { + const index = argv.indexOf(`--${name}`) + const value = index === -1 ? undefined : argv[index + 1] + return value?.startsWith('--') ? undefined : value +} + +async function git(args: readonly string[]): Promise { + const { stdout } = await execFile('git', [...args], { + cwd: rootPath, + maxBuffer: 64 * 1024 * 1024, + }) + return stdout +} + +export function assertBumpOnly(changedFiles: readonly string[]): void { + const sorted = [...changedFiles].sort() + if ( + sorted.length !== BUMP_FILES.length || + sorted.some((file, i) => file !== BUMP_FILES[i]) + ) { + throw new Error( + '[open-release-pr] the bump must change exactly CHANGELOG.md and package.json.\n' + + " Where: the working tree after applying the derivation job's patch.\n" + + ` Saw: ${sorted.join(', ') || '(no changes)'}.\n` + + ' Fix: re-dispatch with mode release-pr. If it repeats, inspect bump.mts.', + ) + } +} + +async function main(): Promise { + const version = readFlag(process.argv.slice(2), 'version') + if (!version) { + throw new Error( + '[open-release-pr] --version is required.\n' + + " Fix: pass the derivation job's version output through.", + ) + } + const changed = (await git(['diff', '--name-only', 'HEAD'])) + .split('\n') + .filter(Boolean) + assertBumpOnly(changed) + const manifestVersion = ( + JSON.parse(readFileSync(path.join(rootPath, 'package.json'), 'utf8')) as { + version?: string + } + ).version + if (manifestVersion !== version) { + throw new Error( + `[open-release-pr] package.json says ${manifestVersion}, expected ${version}.\n` + + ' Fix: re-dispatch with mode release-pr so the patch and the version output agree.', + ) + } + + const env = resolveReleaseEnv() + const parentSha = (await git(['rev-parse', 'HEAD'])).trim() + const baseTreeSha = (await git(['rev-parse', 'HEAD^{tree}'])).trim() + const files = BUMP_FILES.map(relPath => ({ + content: readFileSync(path.join(rootPath, relPath), 'utf8'), + path: relPath, + })) + const releaseBranch = await openReleaseBranch({ env, parentSha, version }) + let sha: string + try { + sha = await commitViaGithubApi({ + baseTreeSha, + branch: releaseBranch.branch, + files, + message: `chore(release): ${version}`, + parentSha, + repo: env.repo, + token: env.token, + }) + } catch (e) { + await discardReleaseBranch(releaseBranch) + throw e + } + const pullRequest = await openReleasePullRequest(releaseBranch) + process.stdout.write( + `[open-release-pr] committed ${sha.slice(0, 7)} on ${releaseBranch.branch}, ` + + `release PR: ${pullRequest.html_url}\n`, + ) + const summaryPath = process.env['GITHUB_STEP_SUMMARY'] + if (summaryPath) { + appendFileSync( + summaryPath, + `Release PR for ${version}: ${pullRequest.html_url}\n`, + ) + } +} + +const SCRIPT_META: ScriptMeta = { + describe: + 'commits the bump via the release App and opens the release PR into the release line', + help: `Usage: node scripts/release/open-release-pr.mts --version + + --version the version bump.mts derived and wrote into the tree + + The publish-npm workflow runs this in its release-pr mode, after applying + the bump to a clean checkout. It needs RELEASE_APP_TOKEN with contents:write + and pull_requests:write, plus the GitHub Actions environment.`, +} + +if (isMainModule(import.meta.url)) { + runMain(main, SCRIPT_META) +} diff --git a/scripts/release/promote.mts b/scripts/release/promote.mts deleted file mode 100644 index ef3358797..000000000 --- a/scripts/release/promote.mts +++ /dev/null @@ -1,93 +0,0 @@ -#!/usr/bin/env node -/** - * @file Land or discard the bump the run created. This is the last thing the - * publish-npm workflow does, and it runs whether the publish succeeded or not. - * - * Success fast-forwards the release line to the bump commit and deletes the - * throwaway branch. Failure deletes the branch and leaves the release line - * alone, which is what makes a failed run cost nothing but the burned version - * number. - * - * Usage: - * node scripts/release/promote.mts --branch npm-publish-v1.1.155 --sha - * node scripts/release/promote.mts --branch npm-publish-v1.1.155 --discard - */ - -import process from 'node:process' - -import { - discardReleaseBranch, - promoteReleaseBranch, - resolveReleaseEnv, -} from './release-branch.mts' -import { isMainModule } from '../lib/is-main-module.mts' -import { runMain } from '../lib/run-main.mts' - -import type { ScriptMeta } from '../lib/run-main.mts' - -function readFlag(argv: readonly string[], name: string): string | undefined { - const index = argv.indexOf(`--${name}`) - const value = index === -1 ? undefined : argv[index + 1] - return value?.startsWith('--') ? undefined : value -} - -export interface ReleasePromotionDependencies { - readonly discard: typeof discardReleaseBranch - readonly promote: typeof promoteReleaseBranch - readonly resolveEnv: typeof resolveReleaseEnv -} - -export async function runReleasePromotion( - argv: readonly string[], - dependencies: ReleasePromotionDependencies = { - discard: discardReleaseBranch, - promote: promoteReleaseBranch, - resolveEnv: resolveReleaseEnv, - }, -): Promise { - const branch = readFlag(argv, 'branch') - const sha = readFlag(argv, 'sha') - const discard = argv.includes('--discard') - if (!branch || (!discard && !sha)) { - throw new Error( - '[promote] --branch is required; promotion also requires --sha.\n' + - " Where: the publish-npm workflow's landing step.\n" + - ' Saw: a missing flag; wanted the bump branch name and its tip SHA.\n' + - " Fix: pass the bump step's release-branch and sha outputs through.", - ) - } - const env = dependencies.resolveEnv() - // The version is only used in the log line; the branch name carries it. - const releaseBranch = { - branch, - env, - version: branch.replace(/^npm-publish-v/, ''), - } - if (discard) { - await dependencies.discard(releaseBranch) - return - } - await dependencies.promote(releaseBranch, sha!) -} - -async function main(): Promise { - await runReleasePromotion(process.argv.slice(2)) -} - -const SCRIPT_META: ScriptMeta = { - describe: - 'lands or discards the throwaway release branch the bump stage created', - help: `Usage: node scripts/release/promote.mts --branch [--sha ] [--discard] - - --branch the npm-publish-v branch the bump stage opened - --sha that branch's tip commit; required unless --discard is set - --discard delete the branch instead of landing it, which is what a - failed publish run does - - The publish-npm workflow runs this last, whether the publish succeeded or - not. It needs RELEASE_APP_TOKEN and the GitHub Actions environment.`, -} - -if (isMainModule(import.meta.url)) { - runMain(main, SCRIPT_META) -} diff --git a/scripts/release/release-branch.mts b/scripts/release/release-branch.mts index 446013129..5c6fcda31 100644 --- a/scripts/release/release-branch.mts +++ b/scripts/release/release-branch.mts @@ -1,19 +1,7 @@ /** - * @file The throwaway release branch a CI bump lands on, and the two ways it - * ends. - * - * The bump commit never lands on the release line directly. It goes to - * `npm-publish-v`, and only a run that gets all the way through - * staging fast-forwards the release line to that branch tip. A run that fails - * anywhere — build, pack, smoke test, tag, stage — deletes the branch instead, - * so the release line never sees a version that did not ship. - * - * The landing is a ref fast-forward, not a pull request. A fresh bump branch - * has no protected-branch rules to satisfy, so a PR route parks the release - * behind checks it can never pass, and there is nothing to review in a - * machine-generated bump anyway. The fast-forward also preserves the App's - * exact signed SHA — the SHA the release tag already points at — which a - * squash would rewrite. + * @file The `npm-publish-v` branch a CI bump is committed to, and the + * pull request that carries it onto the protected release line. The publish + * run tags whatever commit that PR's merge leaves at the release line's tip. */ import process from 'node:process' @@ -23,15 +11,17 @@ import { createBranchRef, deleteBranchRef, updateBranchRef, + upsertPullRequest, } from './github-api.mts' +import type { PullRequest } from './github-api.mts' + export interface ReleaseEnv { - // The branch a successful publish fast-forwards, i.e. the dispatch branch. + // The branch the release PR targets, i.e. the dispatch branch. readonly releaseLine: string // Repo in `owner/name` form. readonly repo: string - // Release App token with contents:write — the branch refs, the bump commit, - // and the fast-forward that lands it. + // Release App token with contents:write and pull_requests:write. readonly token: string } @@ -42,11 +32,7 @@ export interface ReleaseBranch { } /** - * Resolve the CI release environment. This is the PROMOTE PREFLIGHT: it runs at - * bump time, before anything is built or staged, so a missing token refuses - * while nothing has been paid for. Checking at landing time would put the - * failure after the irreversible registry write, stranding a shipped version on - * a throwaway branch. + * Resolve the CI release environment before any branch is written. */ export function resolveReleaseEnv(): ReleaseEnv { const repo = process.env['GITHUB_REPOSITORY'] @@ -65,7 +51,7 @@ export function resolveReleaseEnv(): ReleaseEnv { `[release-branch] the CI bump is missing ${missing.join(', ')}.\n` + ` Where: the publish-npm workflow's step env, read before anything is built.\n` + ` Wanted: GITHUB_REPOSITORY + GITHUB_REF_NAME, plus a release App token with\n` + - ` contents:write for the branch, the bump commit, and the fast-forward.\n` + + ` contents:write and pull_requests:write for the branch, commit, and PR.\n` + ` Fix: mint the token in the workflow step and pass it as RELEASE_APP_TOKEN.`, ) } @@ -122,42 +108,41 @@ export async function openReleaseBranch( } /** - * The publish succeeded: fast-forward the release line to the branch tip, then - * delete the branch. `force` stays false, so GitHub rejects the advance with 422 - * when the release line moved to a commit this one does not descend from. - * - * Removing the branch is tidiness, never correctness — the version is already - * live and already on the release line by then — so a cleanup failure warns - * instead of failing the run. + * Open (or refresh) the PR that merges the bump into the release line. */ -export async function promoteReleaseBranch( +export async function openReleasePullRequest( releaseBranch: ReleaseBranch, - tipSha: string, -): Promise { +): Promise { const { branch, env, version } = releaseBranch - await updateBranchRef({ - branch: env.releaseLine, + return await upsertPullRequest({ + base: env.releaseLine, + body: releasePullRequestBody(env.releaseLine, version), + head: branch, repo: env.repo, - sha: tipSha, + title: `chore(release): ${version}`, token: env.token, }) - process.stdout.write( - `[release-branch] fast-forwarded ${env.releaseLine} to ${tipSha.slice(0, 7)} ` + - `("chore(release): ${version}") via the release App.\n`, - ) - try { - await deleteBranchRef({ branch, repo: env.repo, token: env.token }) - } catch (e) { - process.stdout.write( - `[release-branch] ${env.releaseLine} is landed, but removing ${branch} failed: ` + - `${e instanceof Error ? e.message : String(e)}. Delete it by hand.\n`, - ) - } +} + +export function releasePullRequestBody( + releaseLine: string, + version: string, +): string { + return [ + `Bumps package.json to ${version} and moves the \`## [Unreleased]\` notes under the ${version} heading.`, + '', + 'To release:', + '', + '1. Review the CHANGELOG section and squash-merge this PR.', + `2. Dispatch **Publish to npm registry** on \`${releaseLine}\` with \`dry-run: false\`. It tags the merge commit, cuts the GitHub release, and stages all three packages.`, + '3. Approve each staged package with `pnpm stage approve`.', + '', + `If ${releaseLine} moves before this merges, re-dispatch with \`mode: release-pr\` to rebuild the bump on the new tip.`, + ].join('\n') } /** - * The publish failed: delete the release branch. The release line is never - * touched, so a rejected run leaves no version bump behind. + * Delete the release branch after a failed bump commit. */ export async function discardReleaseBranch( releaseBranch: ReleaseBranch, @@ -165,7 +150,7 @@ export async function discardReleaseBranch( const { branch, env } = releaseBranch await deleteBranchRef({ branch, repo: env.repo, token: env.token }) process.stdout.write( - `[release-branch] publish failed — removed ${branch}; ` + + `[release-branch] bump failed, removed ${branch}. ` + `${env.releaseLine} untouched.\n`, ) } diff --git a/test/release-open-pr.test.mts b/test/release-open-pr.test.mts new file mode 100644 index 000000000..1b5158587 --- /dev/null +++ b/test/release-open-pr.test.mts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { upsertPullRequest } from '../scripts/release/github-api.mts' +import { assertBumpOnly } from '../scripts/release/open-release-pr.mts' +import { releasePullRequestBody } from '../scripts/release/release-branch.mts' + +const PR_CONFIG = { + apiUrl: 'https://api.example.test', + base: 'v1.x', + body: 'fixture body', + head: 'npm-publish-v1.5.1', + repo: 'fixture/release-cli', + title: 'chore(release): 1.5.1', + token: 'placeholder', +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status }) +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('assertBumpOnly', () => { + it('accepts exactly the changelog and the manifest in any order', () => { + expect(() => assertBumpOnly(['package.json', 'CHANGELOG.md'])).not.toThrow() + }) + + it.each([ + [[]], + [['package.json']], + [['CHANGELOG.md', 'package.json', 'scripts/release/bump.mts']], + [['CHANGELOG.md', 'pnpm-lock.yaml']], + ])('refuses %j', files => { + expect(() => assertBumpOnly(files)).toThrow(/exactly CHANGELOG.md/) + }) +}) + +describe('upsertPullRequest', () => { + it('opens a PR when none is open for the branch pair', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse([])) + .mockResolvedValueOnce( + jsonResponse({ html_url: 'https://example.test/pr/7', number: 7 }, 201), + ) + vi.stubGlobal('fetch', fetchMock) + const pr = await upsertPullRequest(PR_CONFIG) + expect(pr.number).toBe(7) + expect(fetchMock.mock.calls[0]![0]).toBe( + 'https://api.example.test/repos/fixture/release-cli/pulls?base=v1.x&head=fixture%3Anpm-publish-v1.5.1&state=open', + ) + const create = fetchMock.mock.calls[1]! + expect(create[0]).toBe( + 'https://api.example.test/repos/fixture/release-cli/pulls', + ) + expect(create[1].method).toBe('POST') + expect(JSON.parse(create[1].body)).toEqual({ + base: 'v1.x', + body: 'fixture body', + head: 'npm-publish-v1.5.1', + title: 'chore(release): 1.5.1', + }) + }) + + it('refreshes the open PR on a re-run', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + jsonResponse([{ html_url: 'https://example.test/pr/7', number: 7 }]), + ) + .mockResolvedValueOnce( + jsonResponse({ html_url: 'https://example.test/pr/7', number: 7 }), + ) + vi.stubGlobal('fetch', fetchMock) + const pr = await upsertPullRequest(PR_CONFIG) + expect(pr.number).toBe(7) + expect(fetchMock).toHaveBeenCalledTimes(2) + const update = fetchMock.mock.calls[1]! + expect(update[0]).toBe( + 'https://api.example.test/repos/fixture/release-cli/pulls/7', + ) + expect(update[1].method).toBe('PATCH') + }) +}) + +describe('releasePullRequestBody', () => { + it('tells the reviewer how to publish from the release line', () => { + const body = releasePullRequestBody('v1.x', '1.5.1') + expect(body).toContain('1.5.1') + expect(body).toContain('squash-merge') + expect(body).toContain('`v1.x`') + expect(body).toContain('pnpm stage approve') + }) +}) diff --git a/test/release-promote.test.mts b/test/release-promote.test.mts deleted file mode 100644 index edddc5407..000000000 --- a/test/release-promote.test.mts +++ /dev/null @@ -1,79 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' - -import { runReleasePromotion } from '../scripts/release/promote.mts' - -function promotionDependencies() { - return { - discard: vi.fn(async () => {}), - promote: vi.fn(async () => {}), - resolveEnv: vi.fn(() => ({ - releaseLine: 'v1.x', - repo: 'fixture/release-cli', - token: 'placeholder', - })), - } -} - -describe('release promotion lifecycle', () => { - it('discards a failed build branch without a verification SHA', async () => { - const dependencies = promotionDependencies() - await runReleasePromotion( - ['--branch', 'npm-publish-v1.1.177', '--discard'], - dependencies, - ) - expect(dependencies.discard).toHaveBeenCalledExactlyOnceWith({ - branch: 'npm-publish-v1.1.177', - env: dependencies.resolveEnv.mock.results[0]!.value, - version: '1.1.177', - }) - expect(dependencies.promote).not.toHaveBeenCalled() - }) - - it('promotes the verified bump SHA on successful staging', async () => { - const dependencies = promotionDependencies() - await runReleasePromotion( - ['--branch', 'npm-publish-v1.1.177', '--sha', 'abc1234'], - dependencies, - ) - expect(dependencies.promote).toHaveBeenCalledExactlyOnceWith( - { - branch: 'npm-publish-v1.1.177', - env: dependencies.resolveEnv.mock.results[0]!.value, - version: '1.1.177', - }, - 'abc1234', - ) - expect(dependencies.discard).not.toHaveBeenCalled() - }) - - it.each([ - ['--branch', 'npm-publish-v1.1.177'], - ['--sha', 'abc1234'], - ['--branch', '--discard'], - ['--discard'], - ])( - 'rejects missing required inputs before resolving credentials: %j', - async (...argv) => { - const dependencies = promotionDependencies() - await expect( - runReleasePromotion(argv, dependencies), - ).rejects.toBeInstanceOf(Error) - expect(dependencies.resolveEnv).not.toHaveBeenCalled() - expect(dependencies.discard).not.toHaveBeenCalled() - expect(dependencies.promote).not.toHaveBeenCalled() - }, - ) - - it('propagates a failed discard for workflow recovery', async () => { - const dependencies = promotionDependencies() - const failure = new Error('fixture deletion failure') - dependencies.discard.mockRejectedValueOnce(failure) - await expect( - runReleasePromotion( - ['--branch', 'npm-publish-v1.1.177', '--discard'], - dependencies, - ), - ).rejects.toBe(failure) - expect(dependencies.promote).not.toHaveBeenCalled() - }) -}) diff --git a/test/release-workflow.test.mts b/test/release-workflow.test.mts index d3cdcecd5..69bdaacf5 100644 --- a/test/release-workflow.test.mts +++ b/test/release-workflow.test.mts @@ -10,11 +10,20 @@ interface ReleaseStep { uses?: string } +interface ReleaseJob { + if?: string + needs?: string | string[] + permissions: Record + steps: ReleaseStep[] +} + interface ReleaseWorkflow { concurrency: { group: string; 'cancel-in-progress': boolean } jobs: { - land: { steps: ReleaseStep[] } + derive: ReleaseJob + 'release-pr': ReleaseJob verify: { + if: string environment?: string outputs: { sha: string } permissions: Record @@ -38,17 +47,57 @@ const workflow = parse( ) as ReleaseWorkflow describe('v1 release workflow contract', () => { - it('scopes both release App tokens to the current repository', () => { - const mintSteps = Object.values(workflow.jobs) + it('mints the release App token only in the job that installs nothing', () => { + const mintingJobs = Object.entries(workflow.jobs) + .filter(({ 1: job }) => + job.steps.some( + step => step.run === 'node scripts/release/mint-app-token.mjs', + ), + ) + .map(({ 0: name }) => name) + expect(mintingJobs).toEqual(['release-pr']) + const mint = workflow.jobs['release-pr'].steps.find( + step => step.run === 'node scripts/release/mint-app-token.mjs', + ) + expect(mint?.env).toMatchObject({ + PERMISSIONS: '{"contents":"write","pull_requests":"write"}', + REPOSITORIES: '${{ github.event.repository.name }}', + }) + expect( + workflow.jobs['release-pr'].steps.map(step => step.name), + ).not.toContain('Install dependencies') + }) + + it('never writes to the release line from the workflow', () => { + const scripts = Object.values(workflow.jobs) .flatMap(job => job.steps) - .filter(step => step.run === 'node scripts/release/mint-app-token.mjs') - expect(mintSteps).toHaveLength(2) - for (const step of mintSteps) { - expect(step.env).toMatchObject({ - PERMISSIONS: '{"contents":"write"}', - REPOSITORIES: '${{ github.event.repository.name }}', - }) - } + .map(step => step.run ?? '') + expect(scripts.join('\n')).not.toMatch(/promote\.mts|git push/) + expect(Object.keys(workflow.jobs).sort()).toEqual([ + 'derive', + 'publish', + 'release-pr', + 'verify', + ]) + }) + + it('routes each mode to its own jobs and keeps dry runs write-free', () => { + expect(workflow.on.workflow_dispatch.inputs['mode']?.default).toBe( + 'publish', + ) + expect(workflow.jobs.verify.if).toBe("${{ inputs.mode == 'publish' }}") + expect(workflow.jobs.derive.if).toBe("${{ inputs.mode == 'release-pr' }}") + expect(workflow.jobs['release-pr'].if).toBe( + "${{ inputs.mode == 'release-pr' && inputs.dry-run == false }}", + ) + expect(workflow.jobs.derive.permissions).toEqual({ contents: 'read' }) + }) + + it('publishes the merged release commit, not whatever HEAD is', () => { + const steps = workflow.jobs.verify.steps.map(step => step.name) + expect(steps.indexOf('Check out the release commit')).toBe( + steps.indexOf('Checkout source') + 1, + ) }) it('uses the migrated trusted publisher environment', () => { @@ -70,9 +119,9 @@ describe('v1 release workflow contract', () => { expect(workflow.jobs.publish.if).toBe('${{ inputs.dry-run == false }}') }) - it('retains bump metadata when verification fails', () => { + it('tags the commit verify built', () => { expect(workflow.jobs.verify.outputs.sha).toBe( - '${{ steps.release-meta.outputs.sha || steps.bump.outputs.sha }}', + '${{ steps.release-meta.outputs.sha }}', ) }) diff --git a/test/script-run-main.test.mts b/test/script-run-main.test.mts index 06a5edc4d..dfbae02f7 100644 --- a/test/script-run-main.test.mts +++ b/test/script-run-main.test.mts @@ -201,7 +201,7 @@ describe('entry scripts', () => { 'scripts/lint.mts', 'scripts/update.mts', 'scripts/release/bump.mts', - 'scripts/release/promote.mts', + 'scripts/release/open-release-pr.mts', ] // Bare `node .mts` needs native type stripping, which landed in From 10bcf25436d571e7312ba28a41020b80cde136fe Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 7 Oct 2026 07:14:01 +0200 Subject: [PATCH 2/2] fix(release): refresh the release PR in place and close superseded ones A re-run used to reset npm-publish-vX.Y.Z to the base before writing the new commit. For a moment the head had no diff, which GitHub treats as a reason to close the open PR. The branch is now left alone and the new commit is force-moved onto it in one step. The failure cleanup only deletes a branch this run created. A re-dispatch that derives a different version used to leave the old npm-publish-v* PR open and mergeable. open-release-pr now closes those and deletes their branches. The publish job also names its mode in its own condition, and the release-commit walk no longer dies on the commit that created package.json. --- .github/workflows/publish-npm.yml | 4 +- scripts/release/github-api.mts | 72 ++++++++++++++++++++++++- scripts/release/open-release-pr.mts | 12 ++++- scripts/release/release-branch.mts | 38 +++++++++----- test/release-open-pr.test.mts | 81 ++++++++++++++++++++++++++++- test/release-workflow.test.mts | 4 +- 6 files changed, 190 insertions(+), 21 deletions(-) diff --git a/.github/workflows/publish-npm.yml b/.github/workflows/publish-npm.yml index e49709a38..32416a7ca 100644 --- a/.github/workflows/publish-npm.yml +++ b/.github/workflows/publish-npm.yml @@ -163,7 +163,7 @@ jobs: set -euo pipefail for COMMIT in $(git rev-list --first-parent --max-count=500 HEAD -- package.json); do VERSION=$(git show "$COMMIT:package.json" | jq -r .version) - PREVIOUS=$(git show "$COMMIT^:package.json" | jq -r .version) + PREVIOUS=$(git show "$COMMIT^:package.json" 2>/dev/null | jq -r .version || true) if [ "$VERSION" != "$PREVIOUS" ]; then git checkout -q --detach "$COMMIT" echo "Releasing $VERSION from $(git log -1 --format='%h %s')." @@ -534,7 +534,7 @@ jobs: publish: name: Mark and stage needs: verify - if: ${{ inputs.dry-run == false }} + if: ${{ inputs.mode == 'publish' && inputs.dry-run == false }} runs-on: ubuntu-latest # npm's trusted-publisher config pins this GitHub environment name (npm TP # is PER-PACKAGE, not per-branch: the socket / @socketsecurity/cli / diff --git a/scripts/release/github-api.mts b/scripts/release/github-api.mts index f480bb5c5..aa6fd3dba 100644 --- a/scripts/release/github-api.mts +++ b/scripts/release/github-api.mts @@ -149,6 +149,8 @@ export interface CommitViaGithubApiConfig { readonly baseTreeSha: string readonly branch: string readonly files: readonly CommitFile[] + // Move the branch even when the new commit does not descend from its tip. + readonly force?: boolean | undefined readonly message: string // Parent commit SHA, usually `HEAD`. readonly parentSha: string @@ -213,6 +215,7 @@ export async function commitViaGithubApi( await updateBranchRef({ apiUrl: cfg.apiUrl, branch: cfg.branch, + force: cfg.force, repo: cfg.repo, sha: commit!.sha, token: cfg.token, @@ -239,8 +242,9 @@ export interface ReleasePullRequestConfig { /** * Open a PR from `head` into `base`, or return the open one that already - * exists for that pair. A re-run force-resets `head`, so the existing PR picks - * up the new commit by itself and only its title and body need refreshing. + * exists for that pair. A re-run force-moves `head` to the new commit, so the + * existing PR picks it up by itself and only its title and body need + * refreshing. */ export async function upsertPullRequest( config: ReleasePullRequestConfig, @@ -283,3 +287,67 @@ export async function upsertPullRequest( }) return created! } + +export interface ClosePullRequestsConfig { + readonly apiUrl?: string | undefined + // Open PRs into this branch are candidates. + readonly base: string + // Head branches starting with this prefix are closed. + readonly headPrefix: string + // Head branch to keep open. + readonly keepHead: string + readonly repo: string + readonly token: string +} + +/** + * Close the open same-repository PRs into `base` whose head branch starts with + * `headPrefix`, except `keepHead`, and delete their branches. Returns the + * closed PR numbers. + */ +export async function closeSupersededPullRequests( + config: ClosePullRequestsConfig, +): Promise { + const cfg = { __proto__: null, ...config } as ClosePullRequestsConfig + const query = new URLSearchParams({ + base: cfg.base, + per_page: '100', + state: 'open', + }) + const open = await githubRequest< + Array<{ + head: { ref: string; repo: { full_name: string } | null } + number: number + }> + >({ + apiUrl: cfg.apiUrl, + method: 'GET', + path: `/repos/${cfg.repo}/pulls?${query}`, + token: cfg.token, + }) + const superseded = (open ?? []).filter( + pr => + pr.head.repo?.full_name === cfg.repo && + pr.head.ref.startsWith(cfg.headPrefix) && + pr.head.ref !== cfg.keepHead, + ) + for (let i = 0, { length } = superseded; i < length; i += 1) { + const pr = superseded[i]! + // eslint-disable-next-line no-await-in-loop + await githubRequest({ + apiUrl: cfg.apiUrl, + body: { state: 'closed' }, + method: 'PATCH', + path: `/repos/${cfg.repo}/pulls/${pr.number}`, + token: cfg.token, + }) + // eslint-disable-next-line no-await-in-loop + await deleteBranchRef({ + apiUrl: cfg.apiUrl, + branch: pr.head.ref, + repo: cfg.repo, + token: cfg.token, + }) + } + return superseded.map(pr => pr.number) +} diff --git a/scripts/release/open-release-pr.mts b/scripts/release/open-release-pr.mts index 944db98b0..df05ae16d 100644 --- a/scripts/release/open-release-pr.mts +++ b/scripts/release/open-release-pr.mts @@ -18,6 +18,7 @@ import { promisify } from 'node:util' import { commitViaGithubApi } from './github-api.mts' import { + closeSupersededReleasePullRequests, discardReleaseBranch, openReleaseBranch, openReleasePullRequest, @@ -107,16 +108,25 @@ async function main(): Promise { baseTreeSha, branch: releaseBranch.branch, files, + force: true, message: `chore(release): ${version}`, parentSha, repo: env.repo, token: env.token, }) } catch (e) { - await discardReleaseBranch(releaseBranch) + if (releaseBranch.created) { + await discardReleaseBranch(releaseBranch) + } throw e } const pullRequest = await openReleasePullRequest(releaseBranch) + const closed = await closeSupersededReleasePullRequests(releaseBranch) + if (closed.length) { + process.stdout.write( + `[open-release-pr] closed superseded release PRs: ${closed.map(n => `#${n}`).join(', ')}\n`, + ) + } process.stdout.write( `[open-release-pr] committed ${sha.slice(0, 7)} on ${releaseBranch.branch}, ` + `release PR: ${pullRequest.html_url}\n`, diff --git a/scripts/release/release-branch.mts b/scripts/release/release-branch.mts index 5c6fcda31..7aed5d1d4 100644 --- a/scripts/release/release-branch.mts +++ b/scripts/release/release-branch.mts @@ -8,9 +8,9 @@ import process from 'node:process' import { GithubApiError, + closeSupersededPullRequests, createBranchRef, deleteBranchRef, - updateBranchRef, upsertPullRequest, } from './github-api.mts' @@ -27,6 +27,8 @@ export interface ReleaseEnv { export interface ReleaseBranch { readonly branch: string + // False when the branch already existed, so it may carry an open PR. + readonly created: boolean readonly env: ReleaseEnv readonly version: string } @@ -74,10 +76,9 @@ export interface OpenReleaseBranchConfig { } /** - * Create `npm-publish-v` at `parentSha`. Idempotent: a leftover branch - * from an earlier crashed run (create returns 422) is force-reset to - * `parentSha`, so this run's commit lands on a clean lineage off the current - * base. + * Create `npm-publish-v` at `parentSha`. A branch left by an earlier + * run (create returns 422) stays untouched. The caller force-moves it straight + * to the new commit, so an open PR never sees a head with no diff. */ export async function openReleaseBranch( config: OpenReleaseBranchConfig, @@ -96,15 +97,9 @@ export async function openReleaseBranch( if (!(e instanceof GithubApiError) || e.status !== 422) { throw e } - await updateBranchRef({ - branch, - force: true, - repo: env.repo, - sha: cfg.parentSha, - token: env.token, - }) + return { branch, created: false, env, version: cfg.version } } - return { branch, env, version: cfg.version } + return { branch, created: true, env, version: cfg.version } } /** @@ -141,6 +136,23 @@ export function releasePullRequestBody( ].join('\n') } +/** + * Close the release PRs for other versions, which a re-dispatch that derives a + * different version leaves behind. + */ +export async function closeSupersededReleasePullRequests( + releaseBranch: ReleaseBranch, +): Promise { + const { branch, env } = releaseBranch + return await closeSupersededPullRequests({ + base: env.releaseLine, + headPrefix: releaseBranchName(''), + keepHead: branch, + repo: env.repo, + token: env.token, + }) +} + /** * Delete the release branch after a failed bump commit. */ diff --git a/test/release-open-pr.test.mts b/test/release-open-pr.test.mts index 1b5158587..e87f8c189 100644 --- a/test/release-open-pr.test.mts +++ b/test/release-open-pr.test.mts @@ -1,8 +1,14 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { upsertPullRequest } from '../scripts/release/github-api.mts' +import { + closeSupersededPullRequests, + upsertPullRequest, +} from '../scripts/release/github-api.mts' import { assertBumpOnly } from '../scripts/release/open-release-pr.mts' -import { releasePullRequestBody } from '../scripts/release/release-branch.mts' +import { + openReleaseBranch, + releasePullRequestBody, +} from '../scripts/release/release-branch.mts' const PR_CONFIG = { apiUrl: 'https://api.example.test', @@ -85,6 +91,77 @@ describe('upsertPullRequest', () => { }) }) +describe('openReleaseBranch', () => { + const env = { releaseLine: 'v1.x', repo: 'fixture/release-cli', token: 'x' } + + it('leaves an existing branch where it is', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ message: 'exists' }, 422)) + vi.stubGlobal('fetch', fetchMock) + const branch = await openReleaseBranch({ + env, + parentSha: 'abc', + version: '1.5.1', + }) + expect(branch.created).toBe(false) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(fetchMock.mock.calls[0]![1].method).toBe('POST') + }) + + it('reports a freshly created branch', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce(jsonResponse({}, 201))) + const branch = await openReleaseBranch({ + env, + parentSha: 'abc', + version: '1.5.1', + }) + expect(branch.created).toBe(true) + }) +}) + +function pr(number: number, ref: string, fullName = 'fixture/release-cli') { + return { head: { ref, repo: { full_name: fullName } }, number } +} + +describe('closeSupersededPullRequests', () => { + const config = { + apiUrl: 'https://api.example.test', + base: 'v1.x', + headPrefix: 'npm-publish-v', + keepHead: 'npm-publish-v1.6.0', + repo: 'fixture/release-cli', + token: 'placeholder', + } + + it('closes other release PRs and leaves everything else open', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + jsonResponse([ + pr(1, 'npm-publish-v1.5.1'), + pr(2, 'npm-publish-v1.6.0'), + pr(3, 'feature/unrelated'), + pr(4, 'npm-publish-v1.5.2', 'someone/fork'), + ]), + ) + .mockImplementation(async () => jsonResponse({})) + vi.stubGlobal('fetch', fetchMock) + expect(await closeSupersededPullRequests(config)).toEqual([1]) + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(fetchMock.mock.calls[1]![0]).toBe( + 'https://api.example.test/repos/fixture/release-cli/pulls/1', + ) + expect(JSON.parse(fetchMock.mock.calls[1]![1].body)).toEqual({ + state: 'closed', + }) + expect(fetchMock.mock.calls[2]![0]).toBe( + 'https://api.example.test/repos/fixture/release-cli/git/refs/heads/npm-publish-v1.5.1', + ) + expect(fetchMock.mock.calls[2]![1].method).toBe('DELETE') + }) +}) + describe('releasePullRequestBody', () => { it('tells the reviewer how to publish from the release line', () => { const body = releasePullRequestBody('v1.x', '1.5.1') diff --git a/test/release-workflow.test.mts b/test/release-workflow.test.mts index 69bdaacf5..2a0edc996 100644 --- a/test/release-workflow.test.mts +++ b/test/release-workflow.test.mts @@ -116,7 +116,9 @@ describe('v1 release workflow contract', () => { expect(workflow.jobs.verify.environment).toBeUndefined() expect(workflow.jobs.verify.permissions['id-token']).toBeUndefined() expect(workflow.on.workflow_dispatch.inputs['dry-run']?.default).toBe(true) - expect(workflow.jobs.publish.if).toBe('${{ inputs.dry-run == false }}') + expect(workflow.jobs.publish.if).toBe( + "${{ inputs.mode == 'publish' && inputs.dry-run == false }}", + ) }) it('tags the commit verify built', () => {