fix: Revert "feat: compare all branch authors when deciding if a branch is modified"

lib/modules/platform/default-scm.spec.ts

@@ -45,7 +45,7 @@ describe('modules/platform/default-scm', () => {
 
   it('delegate isBranchModified to util/git', async () => {
     git.isBranchModified.mockResolvedValueOnce(true);
-    await defaultGitScm.isBranchModified('branchName', 'main');
+    await defaultGitScm.isBranchModified('branchName');
     expect(git.isBranchModified).toHaveBeenCalledTimes(1);
   });
 });

lib/modules/platform/default-scm.ts

@@ -27,7 +27,7 @@ export class DefaultGitScm implements PlatformScm {
     return git.isBranchConflicted(baseBranch, branch);
   }
 
-  isBranchModified(branchName: string, baseBranch?: string): Promise<boolean> {
-    return git.isBranchModified(branchName, baseBranch);
+  isBranchModified(branchName: string): Promise<boolean> {
+    return git.isBranchModified(branchName);
   }
 }

lib/modules/platform/types.ts

@@ -218,7 +218,7 @@ export interface Platform {
 
 export interface PlatformScm {
   isBranchBehindBase(branchName: string, baseBranch: string): Promise<boolean>;
-  isBranchModified(branchName: string, baseBranch?: string): Promise<boolean>;
+  isBranchModified(branchName: string): Promise<boolean>;
   isBranchConflicted(baseBranch: string, branch: string): Promise<boolean>;
   branchExists(branchName: string): Promise<boolean>;
   getBranchCommit(branchName: string): Promise<CommitSha | null>;

lib/util/git/index.spec.ts

@@ -77,20 +77,6 @@ describe('util/git/index', () => {
     await repo.addConfig('user.email', '[email protected]');
     await repo.commit('custom message');
 
-    await repo.checkoutBranch('renovate/multiple_commits', defaultBranch);
-    await fs.writeFile(base.path + '/commit1', 'commit1');
-    await repo.add(['commit1']);
-    await repo.addConfig('user.email', '[email protected]');
-    await repo.commit('commit1 message');
-    await fs.writeFile(base.path + '/commit2', 'commit2');
-    await repo.add(['commit2']);
-    await repo.addConfig('user.email', '[email protected]');
-    await repo.commit('commit2 message');
-    await fs.writeFile(base.path + '/commit3', 'commit3');
-    await repo.add(['commit3']);
-    await repo.addConfig('user.email', '[email protected]');
-    await repo.commit('commit3 message');
-
     await repo.checkoutBranch('renovate/nested_files', defaultBranch);
     await fs.mkdirp(base.path + '/bin/');
     await fs.writeFile(base.path + '/bin/nested', 'nested');
@@ -289,59 +275,27 @@ describe('util/git/index', () => {
 
     it('should return false when branch is not found', async () => {
       expect(await git.isBranchModified('renovate/not_found')).toBeFalse();
-      expect(
-        await git.isBranchModified('renovate/not_found', defaultBranch)
-      ).toBeFalse();
     });
 
     it('should return false when author matches', async () => {
       expect(await git.isBranchModified('renovate/future_branch')).toBeFalse();
-      expect(
-        await git.isBranchModified('renovate/future_branch', defaultBranch)
-      ).toBeFalse();
+      expect(await git.isBranchModified('renovate/future_branch')).toBeFalse();
     });
 
     it('should return false when author is ignored', async () => {
       git.setUserRepoConfig({
         gitIgnoredAuthors: ['[email protected]'],
       });
       expect(await git.isBranchModified('renovate/custom_author')).toBeFalse();
-      expect(
-        await git.isBranchModified('renovate/custom_author', defaultBranch)
-      ).toBeFalse();
-    });
-
-    it('should return false if every author is ignored with multiple commits', async () => {
-      git.setUserRepoConfig({
-        gitIgnoredAuthors: ['[email protected]', '[email protected]'],
-      });
-      expect(
-        await git.isBranchModified('renovate/multiple_commits', defaultBranch)
-      ).toBeFalse();
     });
 
     it('should return true when custom author is unknown', async () => {
       expect(await git.isBranchModified('renovate/custom_author')).toBeTrue();
-      expect(
-        await git.isBranchModified('renovate/custom_author', defaultBranch)
-      ).toBeTrue();
-    });
-
-    it('should return true if any commit is modified', async () => {
-      git.setUserRepoConfig({
-        gitIgnoredAuthors: ['[email protected]'],
-      });
-      expect(
-        await git.isBranchModified('renovate/multiple_commits', defaultBranch)
-      ).toBeTrue();
     });
 
     it('should return value stored in modifiedCacheResult', async () => {
       modifiedCache.getCachedModifiedResult.mockReturnValue(true);
       expect(await git.isBranchModified('renovate/future_branch')).toBeTrue();
-      expect(
-        await git.isBranchModified('renovate/future_branch', defaultBranch)
-      ).toBeTrue();
     });
   });
 

lib/util/git/index.ts

@@ -603,10 +603,7 @@ export async function isBranchBehindBase(
   }
 }
 
-export async function isBranchModified(
-  branchName: string,
-  baseBranch?: string
-): Promise<boolean> {
+export async function isBranchModified(branchName: string): Promise<boolean> {
   if (!branchExists(branchName)) {
     logger.debug('branch.isModified(): no cache');
     return false;
@@ -626,48 +623,21 @@ export async function isBranchModified(
     return isModified;
   }
 
+  logger.debug('branch.isModified(): using git to calculate');
+
   await syncGit();
-  // Retrieve the commit authors
-  let branchAuthors: string[] = [];
+  // Retrieve the author of the most recent commit
+  let lastAuthor: string | undefined;
   try {
-    if (baseBranch) {
-      logger.debug(
-        `branch.isModified(): using git to calculate authors between ${branchName} and ${baseBranch}`
-      );
-      branchAuthors = [
-        ...new Set(
-          (
-            await git.raw([
-              'log',
-              '--pretty=format:%ae',
-              `origin/${branchName}...origin/${baseBranch}`,
-              '--',
-            ])
-          )
-            .trim()
-            .split('\n')
-        ),
-      ];
-    } else {
-      logger.debug(
-        `branch.isModified(): checking last author of ${branchName}`
-      );
-      branchAuthors = [
-        ...new Set(
-          (
-            await git.raw([
-              'log',
-              '-1',
-              '--pretty=format:%ae',
-              `origin/${branchName}`,
-              '--',
-            ])
-          )
-            .trim()
-            .split('\n')
-        ),
-      ];
-    }
+    lastAuthor = (
+      await git.raw([
+        'log',
+        '-1',
+        '--pretty=format:%ae',
+        `origin/${branchName}`,
+        '--',
+      ])
+    ).trim();
   } catch (err) /* istanbul ignore next */ {
     if (err.message?.includes('fatal: bad revision')) {
       logger.debug(
@@ -676,19 +646,26 @@ export async function isBranchModified(
       );
       throw new Error(REPOSITORY_CHANGED);
     }
-    logger.warn({ err }, 'Error retrieving git authors for isBranchModified');
+    logger.warn({ err }, 'Error checking last author for isBranchModified');
   }
   const { gitAuthorEmail } = config;
-  let branchIsModified = false;
-  for (const author of branchAuthors) {
-    if (author !== gitAuthorEmail && !config.ignoredAuthors.includes(author)) {
-      branchIsModified = true;
-    }
+  if (
+    lastAuthor === gitAuthorEmail ||
+    config.ignoredAuthors.some((ignoredAuthor) => lastAuthor === ignoredAuthor)
+  ) {
+    // author matches - branch has not been modified
+    logger.debug('branch.isModified() = false');
+    config.branchIsModified[branchName] = false;
+    setCachedModifiedResult(branchName, false);
+    return false;
   }
-  logger.debug(`branch.isModified() = ${branchIsModified}`);
-  config.branchIsModified[branchName] = branchIsModified;
-  setCachedModifiedResult(branchName, branchIsModified);
-  return branchIsModified;
+  logger.debug(
+    { branchName, lastAuthor, gitAuthorEmail },
+    'branch.isModified() = true'
+  );
+  config.branchIsModified[branchName] = true;
+  setCachedModifiedResult(branchName, true);
+  return true;
 }
 
 export async function isBranchConflicted(

lib/workers/repository/config-migration/branch/rebase.ts

@@ -16,7 +16,7 @@ export async function rebaseMigrationBranch(
 ): Promise<string | null> {
   logger.debug('Checking if migration branch needs rebasing');
   const branchName = getMigrationBranchName(config);
-  if (await scm.isBranchModified(branchName, config.baseBranch)) {
+  if (await scm.isBranchModified(branchName)) {
     logger.debug('Migration branch has been edited and cannot be rebased');
     return null;
   }

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

@@ -25,9 +25,7 @@ export async function rebaseOnboardingBranch(
   }
 
   // TODO #7154
-  if (
-    await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch)
-  ) {
+  if (await scm.isBranchModified(config.onboardingBranch!)) {
     logger.debug('Onboarding branch has been edited and cannot be rebased');
     return null;
   }

lib/workers/repository/onboarding/pr/index.ts

@@ -96,9 +96,7 @@ If you need any further assistance then you can also [request help here](${
   if (GlobalConfig.get('dryRun')) {
     // TODO: types (#7154)
     logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`);
-  } else if (
-    await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch)
-  ) {
+  } else if (await scm.isBranchModified(config.onboardingBranch!)) {
     configDesc = emojify(
       `### Configuration\n\n:abcd: Renovate has detected a custom config for this PR. Feel free to ask for [help](${
         config.productLinks!.help

lib/workers/repository/update/branch/index.ts

@@ -190,10 +190,7 @@ export async function processBranch(
       }
 
       logger.debug('Checking if PR has been edited');
-      const branchIsModified = await scm.isBranchModified(
-        config.branchName,
-        config.baseBranch
-      );
+      const branchIsModified = await scm.isBranchModified(config.branchName);
       if (branchPr) {
         logger.debug('Found existing branch PR');
         if (branchPr.state !== 'open') {

lib/workers/repository/update/branch/reuse.ts

@@ -59,7 +59,7 @@ export async function shouldReuseExistingBranch(
     if (await scm.isBranchBehindBase(branchName, baseBranch)) {
       logger.debug(`Branch is behind base branch and needs rebasing`);
       // We can rebase the branch only if no PR or PR can be rebased
-      if (await scm.isBranchModified(branchName, baseBranch)) {
+      if (await scm.isBranchModified(branchName)) {
         logger.debug('Cannot rebase branch as it has been modified');
         result.reuseExistingBranch = true;
         result.isModified = true;
@@ -80,7 +80,7 @@ export async function shouldReuseExistingBranch(
   if (result.isConflicted) {
     logger.debug('Branch is conflicted');
 
-    if ((await scm.isBranchModified(branchName, baseBranch)) === false) {
+    if ((await scm.isBranchModified(branchName)) === false) {
       logger.debug(`Branch is not mergeable and needs rebasing`);
       if (config.rebaseWhen === 'never') {
         logger.debug('Rebasing disabled by config');

lib/workers/repository/update/pr/automerge.ts

@@ -82,7 +82,7 @@ export async function checkAutoMerge(
     };
   }
   // Check if it's been touched
-  if (await scm.isBranchModified(branchName, config.baseBranch)) {
+  if (await scm.isBranchModified(branchName)) {
     logger.debug('PR is ready for automerge but has been modified');
     return {
       automerged: false,