Skip to content

ci(release): ship v1.x versions through a release PR - #1579

Merged
Martin Torp (mtorp) merged 2 commits into
v1.xfrom
martin/v1x-release-pr-flow
Oct 7, 2026
Merged

Martin Torp (mtorp) merged 2 commits into
v1.xfrom
martin/v1x-release-pr-flow

Conversation

@mtorp

@mtorp Martin Torp (mtorp) commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

v1.x now requires pull requests. The publish workflow's last job tried to move v1.x straight to the bump commit with a ref update, and GitHub now rejects that with "Changes must be made through a pull request." By that point the tag, GitHub release and npm stages already existed, so run 37430526096 shipped 1.5.0 but left v1.x on 1.4.2. The 1.4.2 run failed the same way and was landed by hand in #1577.

This PR changes the flow so the workflow never writes to v1.x. A version bump now reaches the branch through a normal reviewed PR.

New flow

  1. Dispatch Publish to npm registry on v1.x with mode: release-pr. With dry-run: true (the default) it only prints the next version. With dry-run: false it opens a chore(release): X.Y.Z PR from npm-publish-vX.Y.Z into v1.x. Re-running it moves the same branch to a fresh commit, so the open PR is refreshed in place. If the re-run derives a different version, the older npm-publish-v* release PR is closed and its branch deleted.
  2. Review the changelog section and squash-merge the PR.
  3. Dispatch with mode: publish and dry-run: false. It checks out the newest commit that changed the package.json version, which is the merged release PR, and builds, tags, releases and stages it the same way as before. Commits merged after the release PR wait for the next release. An already-published version is still refused by release:preflight.
  4. Approve each stage with pnpm stage approve, same as today.

Credential boundaries

  • derive runs pnpm install and bump.mts and holds no write credential. It hands the bump on as a patch.
  • release-pr installs nothing. It applies the patch, refuses anything that touches files other than CHANGELOG.md and package.json, mints the release App token, commits through the API (so the commit is signed), and opens the PR.
  • verify and publish no longer touch the App token at all. Before, verify held a contents:write token in the same job as pnpm install.

The npm trusted publisher bindings don't change. They still point at publish-npm.yml plus the publish-npm environment.

Before the first run

  • The release App installation needs Pull requests: Read and write. mint-app-token.mjs checks the grant before it mints a token, so if that's missing the run stops early with a link to the settings page.
  • The App key is still a repo-level secret (zizmor flags secrets-outside-env). As a follow-up, consider moving it into an environment that only allows v1.x.

Tests

  • test/release-workflow.test.mts now checks that only the release-pr job mints the App token, that no job writes to the release line, and how each mode routes to its jobs.
  • New test/release-open-pr.test.mts covers the two-file patch guard and the create-or-refresh PR logic.
  • pnpm run check:tsc passes. Lint is clean apart from a formatting issue in scripts/ci/setup-node.mjs that is already on v1.x.

A separate PR will clean up the two broken releases (1.4.2 and 1.5.0).

Expected reviewer effort: Full code review


Note

Medium Risk
Changes the production v1.x release path and GitHub App permissions; mistakes could block releases or tag the wrong commit, though dry-run defaults and workflow contract tests reduce exposure.

Overview
v1.x npm releases now go through a reviewed release PR instead of the workflow fast-forwarding v1.x after publish (which broke on branch protection).

The Publish to npm registry workflow adds a mode input: release-pr derives the next version with bump.mts, exports a two-file patch from a new derive job, and release-pr applies it without pnpm install, mints the release App token (now with pull_requests: write), commits via open-release-pr.mts, and opens/refreshes a chore(release): X.Y.Z PR from npm-publish-vX.Y.Z. publish mode keeps verify/publish but verify no longer bumps in-run; on real publishes it checks out the newest first-parent commit that changed package.json version (the merged release PR). The land job and promote.mts are removed.

bump.mts only rewrites the working tree; GitHub writes moved to github-api.mts (upsertPullRequest) and release-branch.mts (PR instead of fast-forward). Docs in CLAUDE.md and workflow comments match the two-step flow. Tests assert credential boundaries and the new PR path.

Reviewed by Cursor Bugbot for commit fce54f6. Configure here.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[agent] Approving. The split into derive (installs, no write credential) and release-pr (App token, installs nothing, two-file patch guard) is a real improvement over verify holding contents:write next to pnpm install. I also like publishing the newest version-changing commit with release:preflight as the backstop. None of the points below block the merge.

Worth addressing before merge

  1. Release PR goes stale if v1.x moves before the merge. A squash merge of an out-of-date release PR ships everything that landed on v1.x after the PR was opened. Those commits never went through bump.mts, so a feat: could ship inside a patch release, and commits without changelog entries won't show up in the version's section. The PR body tells the reviewer to re-dispatch, but nothing enforces it. Cheapest fix: require branches to be up to date before merging on v1.x, or add a check in verify that the release commit's parent is the commit the bump was derived from (the parent of the App commit on npm-publish-vX.Y.Z).
  2. A re-run probably opens a new PR rather than refreshing the old one. openReleaseBranch force-resets an existing npm-publish-vX.Y.Z to parentSha before the new commit is written. For a moment the head has no commits beyond base, and I believe GitHub auto-closes an open PR in that state. upsertPullRequest then finds no open PR (state=open) and creates a new one. That still works, but it doesn't match "refreshes the same PR" in the description and docs, and review comments end up on a closed PR. I haven't confirmed this on a live repo. Either create the new commit on the branch directly with the reset as the fallback, or update the wording.
  3. Superseded release PRs stay open. If a feat: lands and the re-dispatch derives a different version, a new npm-publish-v1.6.0 PR opens and the old npm-publish-v1.5.1 PR stays open and mergeable. Consider closing other open npm-publish-v* PRs into the release line in open-release-pr.mts, or at least mention it in the PR body.

Nits

  1. mode: publish with dry-run: true packs HEAD, while the real run packs the located release commit. A dry run would be a more faithful preview if it also ran the locate step and logged which commit and version it would ship (it can still pack HEAD if you prefer).
  2. publish only stays out of release-pr mode because verify is skipped and implicit success() propagates that. Adding inputs.mode == 'publish' && to publish.if makes this explicit and avoids surprises if someone adds always() later.
  3. In "Check out the release commit", git show "$COMMIT^:package.json" fails under set -e with an unclear git error if the walk reaches the commit that created package.json. That's unlikely within 500 commits, so it's cosmetic.

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.
@mtorp

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Here is what I did with each point. Fixes are in 10bcf25.

  1. Stale release PR: not changed in code. A squash merge only applies the PR's two-file diff, so it can't pull in anything unreviewed. Any v1.x commit that adds a changelog entry also edits ## [Unreleased], so GitHub blocks the stale PR with a conflict until it is re-dispatched. The remaining gap is a feat: landing with no changelog entry. Closing that is a branch protection choice ("require branches to be up to date" on v1.x), not something the workflow can enforce, so I left it to the repo settings.
  2. Re-run opens a new PR: fixed. Good catch. openReleaseBranch no longer resets an existing branch. It leaves it alone and the new commit is force-moved onto it in one step, so the head never has zero diff. The failure cleanup now only deletes a branch the run created. I couldn't confirm the auto-close on a live repo either, but the new order is safe either way.
  3. Superseded release PRs: fixed. open-release-pr.mts now closes other open same-repo npm-publish-v* PRs into the release line and deletes their branches. Tests cover which PRs get closed and which are left alone. The PR body describes it.
  4. Dry run packs HEAD: not changed. That is on purpose. It lets someone check that a branch builds and packs before any release PR is merged. Running the locate step there would make a dry run fail until a release PR exists.
  5. Explicit mode on publish: fixed. publish.if now includes inputs.mode == 'publish', and the workflow test asserts it.
  6. git show "$COMMIT^:package.json": fixed. A missing previous manifest now counts as an empty version, so the commit that created package.json is treated as a version change instead of aborting under set -e.

Workflow and release tests pass and tsgo is clean. The only lint failure left is the scripts/ci/setup-node.mjs formatting issue already on v1.x.

@mtorp
Martin Torp (mtorp) merged commit e27da67 into v1.x Oct 7, 2026
15 checks passed
@mtorp
Martin Torp (mtorp) deleted the martin/v1x-release-pr-flow branch October 7, 2026 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants