fix: disable branch reuse when other rangeStrategies are used along w…

…ith update-lockfile

lib/workers/branch/reuse.spec.ts

@@ -1,7 +1,7 @@
 import { git, platform } from '../../../test/util';
-import type { RenovateConfig } from '../../config/types';
 import { Pr } from '../../platform';
 import { PrState } from '../../types';
+import type { BranchConfig } from '../types';
 import { shouldReuseExistingBranch } from './reuse';
 
 jest.mock('../../util/git');
@@ -13,12 +13,13 @@ describe('workers/branch/reuse', () => {
       state: PrState.Open,
       title: 'any',
     };
-    let config: RenovateConfig;
+    let config: BranchConfig;
     beforeEach(() => {
       config = {
         branchName: 'renovate/some-branch',
         rebaseLabel: 'rebase',
         rebaseWhen: 'behind-base-branch',
+        upgrades: [],
       };
       jest.resetAllMocks();
     });
@@ -43,6 +44,50 @@ describe('workers/branch/reuse', () => {
       expect(res.reuseExistingBranch).toBe(true);
     });
 
+    it('returns false if does not need rebasing but has upgrades that need lockfile maintenance along with upgrades that do not', async () => {
+      config.upgrades = [
+        {
+          packageFile: 'package.json',
+          rangeStrategy: 'replace',
+          branchName: 'current',
+        },
+        {
+          packageFile: 'package.json',
+          rangeStrategy: 'update-lockfile',
+          branchName: 'current',
+        },
+      ];
+      git.branchExists.mockReturnValueOnce(true);
+      platform.getBranchPr.mockResolvedValueOnce({
+        ...pr,
+        isConflicted: false,
+      });
+      const res = await shouldReuseExistingBranch(config);
+      expect(res.reuseExistingBranch).toBe(false);
+    });
+
+    it('returns true if does not need rebasing and lockfile update is on different packages', async () => {
+      config.upgrades = [
+        {
+          packageFile: 'package.json',
+          rangeStrategy: 'replace',
+          branchName: 'current',
+        },
+        {
+          packageFile: 'subpackage/package.json',
+          rangeStrategy: 'update-lockfile',
+          branchName: 'current',
+        },
+      ];
+      git.branchExists.mockReturnValueOnce(true);
+      platform.getBranchPr.mockResolvedValueOnce({
+        ...pr,
+        isConflicted: false,
+      });
+      const res = await shouldReuseExistingBranch(config);
+      expect(res.reuseExistingBranch).toBe(true);
+    });
+
     it('returns false if does not need rebasing but has postUpdateOptions', async () => {
       config.postUpdateOptions = ['yarnDedupeFewer'];
       git.branchExists.mockReturnValueOnce(true);

lib/workers/branch/reuse.ts

@@ -1,16 +1,16 @@
 import { getGlobalConfig } from '../../config/global';
-import type { RenovateConfig } from '../../config/types';
 import { logger } from '../../logger';
 import { platform } from '../../platform';
 import { branchExists, isBranchModified, isBranchStale } from '../../util/git';
+import type { BranchConfig } from '../types';
 
 type ParentBranch = {
   reuseExistingBranch: boolean;
   isModified?: boolean;
 };
 
 export async function shouldReuseExistingBranch(
-  config: RenovateConfig
+  config: BranchConfig
 ): Promise<ParentBranch> {
   const { branchName } = config;
   // Check if branch exists
@@ -87,12 +87,39 @@ export async function shouldReuseExistingBranch(
   }
   logger.debug(`Branch does not need rebasing`);
 
-  // When postUpdateOptions are defined, some lockfiles might be changed after processing packages
-  // This shouldn't be a problem but if the PR is already created and the package.json didn't change
-  // the second pass will only commit the files changed by postUpdateOptions (#10050)
-  // To avoid this we don't reuse branches is postUpdateOptions are present
-  // We are saved from the double push by a check at commit time that won't push identical contents
-  if (config.postUpdateOptions?.length > 0) {
+  // Branches can get in an inconsistent state if "update-lockfile" is used at the same time as other strategies
+  // On the first execution, everything is executed, but if on a second execution the package.json modification is
+  // skipped but the lockfile update is executed, the lockfile will have a different result than if it was executed
+  // along with the changes to the package.json. Thus ending up with an incomplete branch update
+  // This is why we are skipping branch reuse in this case (#10050)
+  const groupedByPackageFile = {};
+  for (const upgrade of config.upgrades) {
+    groupedByPackageFile[upgrade.packageFile] =
+      groupedByPackageFile[upgrade.packageFile] || new Set();
+    groupedByPackageFile[upgrade.packageFile].add(upgrade.rangeStrategy);
+
+    if (
+      groupedByPackageFile[upgrade.packageFile].size > 1 &&
+      groupedByPackageFile[upgrade.packageFile].has('update-lockfile')
+    ) {
+      logger.debug(
+        `Detected multiple rangeStrategies along with update-lockfile`
+      );
+      return {
+        reuseExistingBranch: false,
+        isModified: false,
+      };
+    }
+  }
+
+  // Branches can get in an inconsistent state if postUpdateOptions is used.
+  // package.json updates are run conditionally, but postUpdateOptions are run everytime
+  // On the first execution, everything is executed, but if on a second execution the package.json modification is
+  // skipped but the postUpdateOptions is executed, the lockfile will have a different result than if it was executed
+  // along with the changes to the package.json. Thus ending up with an incomplete branch update
+  // This is why we are skipping branch reuse when postUpdateOptions is used (#10050)
+  if ((config.postUpdateOptions as string[])?.length > 0) {
+    logger.debug(`Branch is using postUpdateOptions`);
     return {
       reuseExistingBranch: false,
       isModified: false,