From 15ea1014932f7d5d0a399d08dd1138f73c2d7412 Mon Sep 17 00:00:00 2001 From: Samuel Lukes Date: Wed, 15 Jul 2026 10:50:10 +0200 Subject: [PATCH] fix: make workspace plugins release correctly with separate-pull-requests When a workspace plugin builds a new candidate for a dependency-only version bump and candidate merging is disabled (the default since #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 #2172 --- src/factories/plugin-factory.ts | 1 + src/manifest.ts | 1 + src/plugins/cargo-workspace.ts | 14 ++++-- src/plugins/maven-workspace.ts | 2 +- src/plugins/node-workspace.ts | 14 ++++-- src/plugins/workspace.ts | 47 +++++++++++++++---- .../separate-pull-requests-workspace.ts | 34 ++++++++++++++ 7 files changed, 95 insertions(+), 18 deletions(-) diff --git a/src/factories/plugin-factory.ts b/src/factories/plugin-factory.ts index fc7754912..ebf575111 100644 --- a/src/factories/plugin-factory.ts +++ b/src/factories/plugin-factory.ts @@ -39,6 +39,7 @@ export interface PluginFactoryOptions { repositoryConfig: RepositoryConfig; manifestPath: string; separatePullRequests?: boolean; + labels?: string[]; // node options alwaysLinkLocal?: boolean; diff --git a/src/manifest.ts b/src/manifest.ts index 18343780b..c05355709 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -414,6 +414,7 @@ export class Manifest { repositoryConfig: this.repositoryConfig, manifestPath: this.manifestPath, separatePullRequests: this.separatePullRequests, + labels: this.labels, }) ); this.pullRequestOverflowHandler = new FilePullRequestOverflowHandler( diff --git a/src/plugins/cargo-workspace.ts b/src/plugins/cargo-workspace.ts index 0a6efe6f2..6a05f6b1b 100644 --- a/src/plugins/cargo-workspace.ts +++ b/src/plugins/cargo-workspace.ts @@ -267,9 +267,15 @@ export class CargoWorkspace extends WorkspacePlugin { const strategy = this.strategiesByPath[updatedPackage.path]; const latestRelease = this.releasesByPath[updatedPackage.path]; const basePullRequest = strategy - ? await strategy.buildReleasePullRequest([], latestRelease, false, [], { - newVersion: version, - }) + ? await strategy.buildReleasePullRequest( + [], + latestRelease, + false, + this.labels, + { + newVersion: version, + } + ) : undefined; if (basePullRequest) { @@ -314,7 +320,7 @@ export class CargoWorkspace extends WorkspacePlugin { }), }, ], - labels: [], + labels: this.labels, headRefName: BranchName.ofTargetBranch(this.targetBranch).toString(), version, draft: false, diff --git a/src/plugins/maven-workspace.ts b/src/plugins/maven-workspace.ts index f08cad3ad..e12fcdb20 100644 --- a/src/plugins/maven-workspace.ts +++ b/src/plugins/maven-workspace.ts @@ -437,7 +437,7 @@ export class MavenWorkspace extends WorkspacePlugin { }), }, ], - labels: [], + labels: this.labels, headRefName: BranchName.ofTargetBranch(this.targetBranch).toString(), version, draft: false, diff --git a/src/plugins/node-workspace.ts b/src/plugins/node-workspace.ts index eec1a2e71..a58f9103d 100644 --- a/src/plugins/node-workspace.ts +++ b/src/plugins/node-workspace.ts @@ -275,9 +275,15 @@ export class NodeWorkspace extends WorkspacePlugin { const latestRelease = this.releasesByPath[updatedPackage.path]; const basePullRequest = strategy - ? await strategy.buildReleasePullRequest([], latestRelease, false, [], { - newVersion, - }) + ? await strategy.buildReleasePullRequest( + [], + latestRelease, + false, + this.labels, + { + newVersion, + } + ) : undefined; if (basePullRequest) { @@ -339,7 +345,7 @@ export class NodeWorkspace extends WorkspacePlugin { }), }, ], - labels: [], + labels: this.labels, headRefName: BranchName.ofTargetBranch(this.targetBranch).toString(), version: newVersion, draft: false, diff --git a/src/plugins/workspace.ts b/src/plugins/workspace.ts index 5e58b73b7..51d6336bc 100644 --- a/src/plugins/workspace.ts +++ b/src/plugins/workspace.ts @@ -36,6 +36,7 @@ export interface WorkspacePluginOptions { updateAllPackages?: boolean; merge?: boolean; logger?: Logger; + labels?: string[]; } export interface AllPackages { @@ -58,6 +59,7 @@ export abstract class WorkspacePlugin extends ManifestPlugin { private updateAllPackages: boolean; private manifestPath: string; private merge: boolean; + protected labels: string[]; constructor( github: Scm, targetBranch: string, @@ -68,6 +70,7 @@ export abstract class WorkspacePlugin extends ManifestPlugin { this.manifestPath = options.manifestPath ?? DEFAULT_RELEASE_PLEASE_MANIFEST; this.updateAllPackages = options.updateAllPackages ?? false; this.merge = options.merge ?? true; + this.labels = options.labels ?? []; } async run( candidates: CandidateReleasePullRequest[] @@ -176,15 +179,41 @@ export abstract class WorkspacePlugin extends ManifestPlugin { newCandidates = await mergePlugin.run(newCandidates); } - const newUpdates = newCandidates[0].pullRequest.updates; - newUpdates.push({ - path: this.manifestPath, - createIfMissing: false, - updater: new ReleasePleaseManifest({ - version: newCandidates[0].pullRequest.version!, - versionsMap: updatedPathVersions, - }), - }); + if (this.merge) { + // All in-scope candidates have been merged into a single pull request, + // so a single update covering every bumped path is attached to it. + const newUpdates = newCandidates[0].pullRequest.updates; + newUpdates.push({ + path: this.manifestPath, + createIfMissing: false, + updater: new ReleasePleaseManifest({ + version: newCandidates[0].pullRequest.version!, + versionsMap: updatedPathVersions, + }), + }); + } else { + // Candidates are kept as separate pull requests (e.g. with + // `separate-pull-requests: true`), so each pull request must carry the + // manifest entry for its own path — attaching every entry to the first + // candidate would strand the other versions when pull requests are + // merged independently. Candidates built from real releases already + // received their manifest entry from the manifest pull request builder; + // only forced version bumps (dependency-only updates) are in + // updatedPathVersions and handled here. + for (const candidate of newCandidates) { + const version = updatedPathVersions.get(candidate.path); + if (version) { + candidate.pullRequest.updates.push({ + path: this.manifestPath, + createIfMissing: false, + updater: new ReleasePleaseManifest({ + version: candidate.pullRequest.version!, + versionsMap: new Map([[candidate.path, version]]), + }), + }); + } + } + } this.logger.info( `Post-processing ${newCandidates.length} in-scope candidates` diff --git a/test/plugins/compatibility/separate-pull-requests-workspace.ts b/test/plugins/compatibility/separate-pull-requests-workspace.ts index 666fdbb5f..0257052d2 100644 --- a/test/plugins/compatibility/separate-pull-requests-workspace.ts +++ b/test/plugins/compatibility/separate-pull-requests-workspace.ts @@ -23,12 +23,14 @@ import { mockCommits, safeSnapshot, stubFilesFromFixtures, + assertHasUpdate, assertHasUpdates, } from '../../helpers'; import {Version} from '../../../src/version'; import {PackageJson} from '../../../src/updaters/node/package-json'; import {expect} from 'chai'; import {Changelog} from '../../../src/updaters/changelog'; +import {ReleasePleaseManifest} from '../../../src/updaters/release-please-manifest'; const sandbox = sinon.createSandbox(); const fixturesPath = './test/fixtures/plugins/node-workspace'; @@ -171,6 +173,21 @@ describe('Plugin compatibility', () => { ) as Update ).updater as Changelog; expect(updaterA.version.toString()).to.eql('1.0.1'); + // the pull request must carry the pending-release label, otherwise + // it is invisible to the tagging phase after being merged + expect(pullRequest1.labels).to.eql(['autorelease: pending']); + // the pull request must update its own manifest entry, otherwise the + // manifest falls out of sync when it is merged independently + const manifestUpdaterA = ( + assertHasUpdate( + pullRequest1.updates, + '.release-please-manifest.json', + ReleasePleaseManifest + ) as Update + ).updater as ReleasePleaseManifest; + expect( + manifestUpdaterA.versionsMap?.get('packages/node1')?.toString() + ).to.eql('1.0.1'); const pullRequest2 = pullRequests[1]; safeSnapshot(pullRequest2.body.toString()); @@ -182,6 +199,17 @@ describe('Plugin compatibility', () => { ) as Update ).updater as Changelog; expect(updaterB.version.toString()).to.eql('1.0.1'); + expect(pullRequest2.labels).to.eql(['autorelease: pending']); + const manifestUpdaterB = ( + assertHasUpdate( + pullRequest2.updates, + '.release-please-manifest.json', + ReleasePleaseManifest + ) as Update + ).updater as ReleasePleaseManifest; + expect( + manifestUpdaterB.versionsMap?.get('packages/node2')?.toString() + ).to.eql('1.0.1'); const pullRequest3 = pullRequests[2]; safeSnapshot(pullRequest3.body.toString()); @@ -193,6 +221,12 @@ describe('Plugin compatibility', () => { ) as Update ).updater as Changelog; expect(updaterC.version.toString()).to.eql('1.1.0'); + expect(pullRequest3.labels).to.eql(['autorelease: pending']); + assertHasUpdate( + pullRequest3.updates, + '.release-please-manifest.json', + ReleasePleaseManifest + ); }); }); });