feat(onboarding): merge onboardingBranch into baseBranch (#20893)

Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

lib/util/cache/repository/types.ts

@@ -27,9 +27,10 @@ export interface BranchUpgradeCache {
 }
 
 export interface OnboardingBranchCache {
-  onboardingBranch: string;
   defaultBranchSha: string;
   onboardingBranchSha: string;
+  isConflicted: boolean;
+  isModified: boolean;
 }
 
 export interface PrCache {

lib/util/git/index.spec.ts

@@ -344,6 +344,12 @@ describe('util/git/index', () => {
       expect(merged.all).toContain('renovate/future_branch');
     });
 
+    it('does not push if localOnly=true', async () => {
+      const pushSpy = jest.spyOn(SimpleGit.prototype, 'push');
+      await git.mergeBranch('renovate/future_branch', true);
+      expect(pushSpy).toHaveBeenCalledTimes(0);
+    });
+
     it('should throw if branch merge throws', async () => {
       await expect(git.mergeBranch('not_found')).rejects.toThrow();
     });

lib/util/git/index.ts

@@ -774,7 +774,10 @@ export async function deleteBranch(branchName: string): Promise<void> {
   delete config.branchCommits[branchName];
 }
 
-export async function mergeBranch(branchName: string): Promise<void> {
+export async function mergeBranch(
+  branchName: string,
+  localOnly = false
+): Promise<void> {
   let status: StatusResult | undefined;
   try {
     await syncGit();
@@ -790,8 +793,13 @@ export async function mergeBranch(branchName: string): Promise<void> {
       ])
     );
     status = await git.status();
-    await gitRetry(() => git.merge(['--ff-only', branchName]));
-    await gitRetry(() => git.push('origin', config.currentBranch));
+    if (localOnly) {
+      // merge commit, don't push to origin
+      await gitRetry(() => git.merge([branchName]));
+    } else {
+      await gitRetry(() => git.merge(['--ff-only', branchName]));
+      await gitRetry(() => git.push('origin', config.currentBranch));
+    }
     incLimitedValue('Commits');
   } catch (err) {
     logger.debug(

lib/workers/repository/onboarding/branch/check.spec.ts

@@ -27,9 +27,10 @@ describe('workers/repository/onboarding/branch/check', () => {
   it('skips normal onboarding check if onboardingCache is valid', async () => {
     cache.getCache.mockReturnValueOnce({
       onboardingBranchCache: {
-        onboardingBranch: 'configure/renovate',
         defaultBranchSha: 'default-sha',
         onboardingBranchSha: 'onboarding-sha',
+        isConflicted: false,
+        isModified: false,
       },
     });
     git.getBranchCommit
@@ -45,9 +46,10 @@ describe('workers/repository/onboarding/branch/check', () => {
   it('continues with normal logic if onboardingCache is invalid', async () => {
     cache.getCache.mockReturnValueOnce({
       onboardingBranchCache: {
-        onboardingBranch: 'configure/renovate',
         defaultBranchSha: 'default-sha',
         onboardingBranchSha: 'onboarding-sha',
+        isConflicted: false,
+        isModified: false,
       },
     });
     scm.getFileList.mockResolvedValue([]);

lib/workers/repository/onboarding/branch/check.ts

@@ -52,8 +52,13 @@ function closedPrExists(config: RenovateConfig): Promise<Pr | null> {
 export async function isOnboarded(config: RenovateConfig): Promise<boolean> {
   logger.debug('isOnboarded()');
   const title = `Action required: Add a Renovate config`;
+
   // Repo is onboarded if global config is bypassing onboarding and does not require a
   // configuration file.
+  // The repo is considered "not onboarded" if:
+  // - An onboarding cache is present, and
+  // - The current default branch SHA matches the default SHA found in the cache
+  // Also if there is a closed pr skip using cache as it is outdated
   if (config.requireConfig === 'optional' && config.onboarding === false) {
     // Return early and avoid checking for config files
     return true;
@@ -63,19 +68,17 @@ export async function isOnboarded(config: RenovateConfig): Promise<boolean> {
     return true;
   }
 
-  const pr = await closedPrExists(config);
+  const closedOnboardingPr = await closedPrExists(config);
   const cache = getCache();
   const onboardingBranchCache = cache?.onboardingBranchCache;
   // if onboarding cache is present and base branch has not been updated branch is not onboarded
   // if closed pr exists then presence of onboarding cache doesn't matter as we need to skip onboarding
   if (
     config.onboarding &&
-    !pr &&
+    !closedOnboardingPr &&
     onboardingBranchCache &&
     onboardingBranchCache.defaultBranchSha ===
-      getBranchCommit(config.defaultBranch!) &&
-    onboardingBranchCache.onboardingBranchSha ===
-      getBranchCommit(config.onboardingBranch!)
+      getBranchCommit(config.defaultBranch!)
   ) {
     logger.debug('Onboarding cache is valid. Repo is not onboarded');
     return false;
@@ -123,7 +126,7 @@ export async function isOnboarded(config: RenovateConfig): Promise<boolean> {
     throw new Error(REPOSITORY_NO_CONFIG);
   }
 
-  if (!pr) {
+  if (!closedOnboardingPr) {
     logger.debug('Found no closed onboarding PR');
     return false;
   }
@@ -136,9 +139,9 @@ export async function isOnboarded(config: RenovateConfig): Promise<boolean> {
   if (!config.suppressNotifications!.includes('onboardingClose')) {
     // ensure PR comment
     await ensureComment({
-      number: pr.number,
+      number: closedOnboardingPr.number,
       topic: `Renovate is disabled`,
-      content: `Renovate is disabled due to lack of config. If you wish to reenable it, you can either (a) commit a config file to your base branch, or (b) rename this closed PR to trigger a replacement onboarding PR.`,
+      content: `Renovate is disabled due to lack of config. If you wish to re-enable it, you can either (a) commit a config file to your base branch, or (b) rename this closed PR to trigger a replacement onboarding PR.`,
     });
   }
   throw new Error(REPOSITORY_CLOSED_ONBOARDING);

lib/workers/repository/onboarding/branch/index.spec.ts

@@ -3,6 +3,7 @@ import {
   RenovateConfig,
   fs,
   getConfig,
+  git,
   mocked,
   platform,
   scm,
@@ -17,17 +18,15 @@ import { logger } from '../../../../logger';
 import type { Pr } from '../../../../modules/platform';
 import * as memCache from '../../../../util/cache/memory';
 import * as _cache from '../../../../util/cache/repository';
+import type { RepoCacheData } from '../../../../util/cache/repository/types';
 import type { FileAddition } from '../../../../util/git/types';
 import { OnboardingState } from '../common';
 import * as _config from './config';
 import * as _onboardingCache from './onboarding-branch-cache';
-import * as _rebase from './rebase';
 import { checkOnboardingBranch } from '.';
 
-const rebase: any = _rebase;
 const configModule: any = _config;
 
-jest.mock('../../../repository/onboarding/branch/rebase');
 jest.mock('../../../../util/cache/repository');
 jest.mock('../../../../util/fs');
 jest.mock('../../../../util/git');
@@ -40,13 +39,6 @@ const onboardingCache = mocked(_onboardingCache);
 describe('workers/repository/onboarding/branch/index', () => {
   describe('checkOnboardingBranch', () => {
     let config: RenovateConfig;
-    const dummyCache = {
-      onboardingBranchCache: {
-        onboardingBranch: 'configure/renovate',
-        defaultBranchSha: 'default-sha',
-        onboardingBranchSha: 'onboarding-sha',
-      },
-    };
 
     beforeEach(() => {
       memCache.init();
@@ -177,7 +169,6 @@ describe('workers/repository/onboarding/branch/index', () => {
     });
 
     it('detects repo is onboarded via file', async () => {
-      cache.getCache.mockReturnValue(dummyCache);
       scm.getFileList.mockResolvedValueOnce(['renovate.json']);
       const res = await checkOnboardingBranch(config);
       expect(res.repoIsOnboarded).toBeTrue();
@@ -261,28 +252,97 @@ describe('workers/repository/onboarding/branch/index', () => {
       await expect(checkOnboardingBranch(config)).rejects.toThrow();
     });
 
-    it('updates onboarding branch', async () => {
-      cache.getCache.mockReturnValue(dummyCache);
+    it('processes onboarding branch', async () => {
       scm.getFileList.mockResolvedValue(['package.json']);
       platform.findPr.mockResolvedValue(null);
       platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
-      rebase.rebaseOnboardingBranch.mockResolvedValueOnce('123test');
       const res = await checkOnboardingBranch(config);
       expect(res.repoIsOnboarded).toBeFalse();
       expect(res.branchList).toEqual(['renovate/configure']);
-      expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
-      expect(onboardingCache.setOnboardingCache).toHaveBeenCalledTimes(1); // update onboarding cache
+      expect(git.mergeBranch).toHaveBeenCalledOnce();
       expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
     });
 
+    it('processes modified onboarding branch and invalidates extract cache', async () => {
+      const dummyCache = {
+        scan: {
+          master: {
+            sha: 'base_sha',
+            configHash: 'hash',
+            packageFiles: {},
+            extractionFingerprints: {},
+          },
+        },
+      } satisfies RepoCacheData;
+      cache.getCache.mockReturnValue(dummyCache);
+      platform.findPr.mockResolvedValue(null);
+      platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
+      git.getBranchCommit
+        .mockReturnValueOnce('default-sha')
+        .mockReturnValueOnce('new-onboarding-sha');
+      onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(true);
+      onboardingCache.hasOnboardingBranchChanged.mockReturnValueOnce(true);
+      onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(false);
+      config.baseBranch = 'master';
+      await checkOnboardingBranch(config);
+      expect(git.mergeBranch).toHaveBeenCalledOnce();
+      expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
+        'default-sha',
+        'new-onboarding-sha',
+        false,
+        true
+      );
+      expect(dummyCache).toMatchObject({
+        scan: {},
+      });
+    });
+
+    it('skips processing conflicted onboarding branch', async () => {
+      scm.getFileList.mockResolvedValue(['package.json']);
+      platform.findPr.mockResolvedValue(null);
+      platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
+      platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
+      git.getBranchCommit
+        .mockReturnValueOnce('default-sha')
+        .mockReturnValueOnce('onboarding-sha');
+      onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(true);
+      onboardingCache.hasOnboardingBranchChanged.mockReturnValueOnce(true);
+      onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(true);
+      await checkOnboardingBranch(config);
+      expect(git.mergeBranch).not.toHaveBeenCalled();
+      expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
+        'default-sha',
+        'onboarding-sha',
+        true,
+        true
+      );
+    });
+
+    it('sets onboarding cache for existing onboarding branch', async () => {
+      scm.getFileList.mockResolvedValue(['package.json']);
+      platform.findPr.mockResolvedValue(null);
+      platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
+      git.getBranchCommit
+        .mockReturnValueOnce('default-sha')
+        .mockReturnValueOnce('onboarding-sha');
+      onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(false);
+      await checkOnboardingBranch(config);
+      expect(git.mergeBranch).toHaveBeenCalled();
+      expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
+        'default-sha',
+        'onboarding-sha',
+        false,
+        false
+      );
+    });
+
     describe('tests onboarding rebase/retry checkbox handling', () => {
       beforeEach(() => {
         GlobalConfig.set({ platform: 'github' });
         config.onboardingRebaseCheckbox = true;
         OnboardingState.prUpdateRequested = false;
         scm.getFileList.mockResolvedValueOnce(['package.json']);
         platform.findPr.mockResolvedValueOnce(null);
-        rebase.rebaseOnboardingBranch.mockResolvedValueOnce(null);
       });
 
       it('detects unsupported platfom', async () => {
@@ -296,7 +356,7 @@ describe('workers/repository/onboarding/branch/index', () => {
           `Platform '${pl}' does not support extended markdown`
         );
         expect(OnboardingState.prUpdateRequested).toBeTrue();
-        expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
+        expect(git.mergeBranch).toHaveBeenCalledOnce();
         expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
       });
 
@@ -310,7 +370,7 @@ describe('workers/repository/onboarding/branch/index', () => {
           `No rebase checkbox was found in the onboarding PR`
         );
         expect(OnboardingState.prUpdateRequested).toBeTrue();
-        expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
+        expect(git.mergeBranch).toHaveBeenCalledOnce();
         expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
       });
 
@@ -324,8 +384,7 @@ describe('workers/repository/onboarding/branch/index', () => {
           `Manual onboarding PR update requested`
         );
         expect(OnboardingState.prUpdateRequested).toBeTrue();
-        ``;
-        expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
+        expect(git.mergeBranch).toHaveBeenCalledOnce();
         expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
       });
 
@@ -336,7 +395,7 @@ describe('workers/repository/onboarding/branch/index', () => {
         await checkOnboardingBranch(config);
 
         expect(OnboardingState.prUpdateRequested).toBeFalse();
-        expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
+        expect(git.mergeBranch).toHaveBeenCalledOnce();
         expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
       });
     });