fix: make workspace plugins release correctly with separate-pull-requests - #2847
Open
slukes wants to merge 1 commit into
Open
fix: make workspace plugins release correctly with separate-pull-requests#2847slukes wants to merge 1 commit into
slukes wants to merge 1 commit into
Conversation
…ests When a workspace plugin builds a new candidate for a dependency-only version bump and candidate merging is disabled (the default since googleapis#2310 when separate-pull-requests is true), two things break: - The candidate pull request is created without the configured release labels (hardcoded `labels: []`), so after it is merged, findMergedReleasePullRequests filters it out and no tag or GitHub release is ever created for it. The open-PR search is also label filtered, so release-please does not even recognize its own open cascade pull requests. - The manifest entries for every force-bumped path are attached to the first candidate only (a leftover from when candidates were always merged into a single pull request), so independently merged pull requests leave .release-please-manifest.json permanently out of sync, and each subsequent run re-bumps from package.json and opens another doomed pull request one patch higher. Pass the manifest's labels through the plugin factory to the workspace plugins so new candidates carry them, and, when not merging, attach each forced bump's manifest entry to its own candidate pull request instead of attaching all of them to the first one. Fixes googleapis#2172
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #2172 🦕
Problem
Since #2310, workspace plugins default to
merge: falsewhenseparate-pull-requests: trueis configured. On that code path, a candidate created for a dependency-only version bump ("cascade") is broken in two ways — both leftovers from when in-scope candidates were always merged into a single pull request:newCandidate()hardcodeslabels: [](and passes[]tostrategy.buildReleasePullRequest). Undermerge: truethis was harmless because theMergeplugin unions labels from the real candidates, but as a standalone PR the cascade never carriesautorelease: pending. Consequences:findMergedReleasePullRequestsfilters merged PRs by these labels, so after the cascade PR is merged, no tag or GitHub release is ever created for it.findOpenReleasePullRequestsuses the same filter, so release-please does not recognize its own open cascade PRs and force-pushes their branches on every run.newCandidates[0]only (introduced in fix: workspace plugins should update manifest versions #1429, whennewCandidates[0]was by construction the single merged PR). With separate PRs it lands on whichever candidate sorts first alphabetically. Merging any other cascade PR leaves.release-please-manifest.jsonout of sync, and each subsequent run re-bumps from the package manifest and opens another doomed PR one patch higher. (If the first candidate's body happens to be unchanged,maybeUpdateExistingPullRequestskips the push entirely and the manifest update is silently dropped.)Observed in production on a pnpm monorepo: dependency-only releases escalated three patch versions in a single day without a single tag or publish, while the manifest entries for those packages accumulated on an unrelated package's release PR.
Fix
labelsthroughbuildPluginto the workspace plugins (node-workspace,cargo-workspace,maven-workspace), and use them innewCandidate()— both thestrategy.buildReleasePullRequestpath and the fallback literal. Undermerge: truethis is a no-op (label union is unchanged).WorkspacePlugin.run(), keep the existing single combined manifest update whenmergeis enabled; when it is disabled, attach each forced bump's manifest entry to its own candidate pull request. Candidates built from real releases already receive their manifest entry from the manifest PR builder, so nothing is duplicated.Tests
Extended the plugin-compatibility test added in #2310 (
test/plugins/compatibility/separate-pull-requests-workspace.ts) to also assert that each separate pull request carries the pending-release label and the manifest entry for its own path. Without the fix these assertions fail (expected [] to deeply equal [ 'autorelease: pending' ]); with it, the full suite passes (1201 tests).