From 618fe2e4a9dd3e4b3d014a05bf47c0f7c8f80600 Mon Sep 17 00:00:00 2001
From: Tom Coleman <tom@chromatic.com>
Date: Tue, 28 Nov 2023 12:14:45 +1100
Subject: [PATCH 01/11] Refactored squash merge detection to run after git
 tracing

Instead of first checking if the current commit is a PR merge commit, instead, calculate all possible ancestor commits, THEN append the PR head to that list if relevant.

This is a first step towards checking more than one commit being a PR merge commit
---
 node-src/git/getParentCommits.test.ts |  57 +++++++-
 node-src/git/getParentCommits.ts      | 192 +++++++++++++++++---------
 node-src/git/mocks/mock-index.ts      |  42 +++---
 3 files changed, 207 insertions(+), 84 deletions(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index fd551998c..624d80c8c 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -619,7 +619,7 @@ describe('getParentCommits', () => {
       //            /   \
       //  A - B -[C]      F
       //            \   /
-      //             [E]   [main]
+      //             (E)   [main]
       const repository = repositories.simpleLoop;
       await checkoutCommit('E', 'main', repository);
       const client = createClient({
@@ -637,5 +637,60 @@ describe('getParentCommits', () => {
       // This doesn't include 'C' as D "covers" it.
       expectCommitsToEqualNames(parentCommits, ['D'], repository);
     });
+
+    it(`also finds squash merge commits that were on previous commits without builds`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //     B -[D]     [branch]
+      //    /
+      //  A - [C]-<E>-(G) [main]
+      const repository = repositories.longLoop;
+      await checkoutCommit('G', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['C', 'main'],
+          ['D', 'branch'],
+        ],
+        // Talking to GH (etc) tells us that commit E is the merge commit for "branch"
+        prs: [['E', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
+
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      expectCommitsToEqualNames(parentCommits, ['D', 'C'], repository);
+    });
+
+    it(`deals with situations where the last build on the squashed branch isn't the last commit`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //     [B] - D      [branch]
+      //    /
+      //  A -[C]-<E>-(G) [main]
+      const repository = repositories.longLoop;
+      await checkoutCommit('G', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['C', 'main'],
+          ['B', 'branch'],
+        ],
+        // Talking to GH (etc) tells us that commit E is the merge commit for "branch"
+        prs: [['E', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
+
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      // This doesn't include 'C' as D "covers" it.
+      expectCommitsToEqualNames(parentCommits, ['C', 'B'], repository);
+    });
+
+    // it(`deals with situations where squashed branches have no builds`)
+
+    // it(`deals with situations where squashed branches no longer exist in the repo but have a build`)
+
+    // it(`deals with situations where squashed branches no longer exist in the repo and have no build`)
+
+    // it(`deals with situations where squashed branches themselves have squash merge commits`)
   });
 });
diff --git a/node-src/git/getParentCommits.ts b/node-src/git/getParentCommits.ts
index ea0ad362a..b32479cc9 100644
--- a/node-src/git/getParentCommits.ts
+++ b/node-src/git/getParentCommits.ts
@@ -8,11 +8,7 @@ import { localBuildsSpecifier } from '../lib/localBuildsSpecifier';
 export const FETCH_N_INITIAL_BUILD_COMMITS = 20;
 
 const FirstCommittedAtQuery = gql`
-  query FirstCommittedAtQuery(
-    $commit: String!
-    $branch: String!
-    $localBuilds: LocalBuildsSpecifierInput!
-  ) {
+  query FirstCommittedAtQuery($branch: String!, $localBuilds: LocalBuildsSpecifierInput!) {
     app {
       firstBuild(sortByCommittedAt: true, localBuilds: $localBuilds) {
         committedAt
@@ -21,11 +17,6 @@ const FirstCommittedAtQuery = gql`
         commit
         committedAt
       }
-      pullRequest(mergeInfo: { commit: $commit, baseRefName: $branch }) {
-        lastHeadBuild {
-          commit
-        }
-      }
     }
   }
 `;
@@ -38,11 +29,6 @@ interface FirstCommittedAtQueryResult {
       commit: string;
       committedAt: number;
     };
-    pullRequest: {
-      lastHeadBuild: {
-        commit: string;
-      };
-    };
   };
 }
 
@@ -59,6 +45,27 @@ interface HasBuildsWithCommitsQueryResult {
   };
 }
 
+const IsMergeCommitQuery = gql`
+  query IsMergeCommitQuery($commit: String!, $branch: String!) {
+    app {
+      pullRequest(mergeInfo: { commit: $commit, baseRefName: $branch }) {
+        lastHeadBuild {
+          commit
+        }
+      }
+    }
+  }
+`;
+interface IsMergeCommitQueryResult {
+  app: {
+    pullRequest: {
+      lastHeadBuild: {
+        commit: string;
+      };
+    };
+  };
+}
+
 function commitsForCLI(commits: string[]) {
   return commits.map((c) => c.trim()).join(' ');
 }
@@ -102,32 +109,18 @@ async function nextCommits(
   const commits = (await execGitCommand(command)).split('\n').filter(Boolean);
   log.debug(`command output: ${commits}`);
 
-  return (
-    commits
-      // No sense in checking commits we already know about
-      .filter((c) => !commitsWithBuilds.includes(c))
-      .filter((c) => !commitsWithoutBuilds.includes(c))
-      .slice(0, limit)
-  );
-}
+  // Later on we want to know which commits we visited on the way to finding the ancestor commits
+  // The output of the above rev-list commit includes possibly commits with builds so filter them.
+  // NOTE: this list can include irrelevant commits during earlier iterations of step() when we
+  // don't yet have an accurate list of commitsWithBuilds, however we only use it in the final step.
+  const visitedCommitsWithoutBuilds = commits.filter((c) => !commitsWithBuilds.includes(c));
 
-// Which of the listed commits are "maximally descendent":
-// ie c in commits such that there are no descendents of c in commits.
-async function maximallyDescendentCommits({ log }: Pick<Context, 'log'>, commits: string[]) {
-  if (commits.length === 0) {
-    return commits;
-  }
-
-  // <commit>^@ expands to all parents of commit
-  const parentCommits = commits.map((c) => `"${c}^@"`);
-  // List the tree from <commits> not including the tree from <parentCommits>
-  // This just filters any commits that are ancestors of other commits
-  const command = `git rev-list ${commitsForCLI(commits)} --not ${commitsForCLI(parentCommits)}`;
-  log.debug(`running ${command}`);
-  const maxCommits = (await execGitCommand(command)).split('\n').filter(Boolean);
-  log.debug(`command output: ${maxCommits}`);
+  // No sense in checking commits we already know about
+  const candidateCommits = visitedCommitsWithoutBuilds
+    .filter((c) => !commitsWithoutBuilds.includes(c))
+    .slice(0, limit);
 
-  return maxCommits;
+  return { visitedCommitsWithoutBuilds, candidateCommits };
 }
 
 // Exponentially iterate `limit` up to infinity to find a "covering" set of commits with builds
@@ -143,23 +136,25 @@ async function step(
     commitsWithBuilds: string[];
     commitsWithoutBuilds: string[];
   }
-): Promise<string[]> {
+) {
   log.debug(`step: checking ${limit} up to ${firstCommittedAtSeconds}`);
   log.debug(`step: commitsWithBuilds: ${commitsWithBuilds}`);
   log.debug(`step: commitsWithoutBuilds: ${commitsWithoutBuilds}`);
 
-  const candidateCommits = await nextCommits({ log }, limit, {
+  const { candidateCommits, visitedCommitsWithoutBuilds } = await nextCommits({ log }, limit, {
     firstCommittedAtSeconds,
     commitsWithBuilds,
     commitsWithoutBuilds,
   });
 
-  log.debug(`step: candidateCommits: ${candidateCommits}`);
+  log.debug(
+    `step: candidateCommits: ${candidateCommits}, visitedCommitsWithoutBuilds: ${visitedCommitsWithoutBuilds}`
+  );
 
   // No more commits uncovered commitsWithBuilds!
   if (candidateCommits.length === 0) {
     log.debug('step: no candidateCommits; we are done');
-    return commitsWithBuilds;
+    return { commitsWithBuilds, commitsWithoutBuilds, visitedCommitsWithoutBuilds };
   }
 
   const {
@@ -181,6 +176,25 @@ async function step(
   });
 }
 
+// Which of the listed commits are "maximally descendent":
+// ie c in commits such that there are no descendents of c in commits.
+async function maximallyDescendentCommits({ log }: Pick<Context, 'log'>, commits: string[]) {
+  if (commits.length === 0) {
+    return commits;
+  }
+
+  // <commit>^@ expands to all parents of commit
+  const parentCommits = commits.map((c) => `"${c}^@"`);
+  // List the tree from <commits> not including the tree from <parentCommits>
+  // This just filters any commits that are ancestors of other commits
+  const command = `git rev-list ${commitsForCLI(commits)} --not ${commitsForCLI(parentCommits)}`;
+  log.debug(`running ${command}`);
+  const maxCommits = (await execGitCommand(command)).split('\n').filter(Boolean);
+  log.debug(`command output: ${maxCommits}`);
+
+  return maxCommits;
+}
+
 export async function getParentCommits(
   { options, client, git, log }: Context,
   { ignoreLastBuildOnBranch = false } = {}
@@ -190,16 +204,11 @@ export async function getParentCommits(
   // Include the latest build from this branch as an ancestor of the current build
   const { app } = await client.runQuery<FirstCommittedAtQueryResult>(
     FirstCommittedAtQuery,
-    { branch, commit, localBuilds: localBuildsSpecifier({ options, git }) },
+    { branch, localBuilds: localBuildsSpecifier({ options, git }) },
     { retries: 5 } // This query requires a request to an upstream provider which may fail
   );
-  const { firstBuild, lastBuild, pullRequest } = app;
-  log.debug(
-    `App firstBuild: %o, lastBuild: %o, pullRequest: %o`,
-    firstBuild,
-    lastBuild,
-    pullRequest
-  );
+  const { firstBuild, lastBuild } = app;
+  log.debug(`App firstBuild: %o, lastBuild: %o`, firstBuild, lastBuild);
 
   if (!firstBuild) {
     log.debug('App has no builds, returning []');
@@ -233,15 +242,41 @@ export async function getParentCommits(
     }
   }
 
+  // Get a "covering" set of commits that have builds. This is a set of commits
+  // such that any ancestor of HEAD is either:
+  //   - in commitsWithBuilds
+  //   - an ancestor of a commit in commitsWithBuilds
+  //   - has no build
+  const { commitsWithBuilds } = await step(
+    { options, client, log, git },
+    FETCH_N_INITIAL_BUILD_COMMITS,
+    {
+      firstCommittedAtSeconds: firstBuild.committedAt && firstBuild.committedAt / 1000,
+      commitsWithBuilds: initialCommitsWithBuilds,
+      commitsWithoutBuilds: [],
+    }
+  );
+
+  // Check if the current commit is a merge commit
+  const {
+    app: { pullRequest },
+  } = await client.runQuery<IsMergeCommitQueryResult>(
+    IsMergeCommitQuery,
+    { commit, branch },
+    { retries: 5 } // This query requires a request to an upstream provider which may fail
+  );
+
   // Add the most recent build on a (merged) branch as a parent if we think this was the commit that
   // merged the pull request.
   // @see https://www.chromatic.com/docs/branching-and-baselines#squash-and-rebase-merging
+  // let shouldRecurse = false;
   if (pullRequest && pullRequest.lastHeadBuild) {
     if (await commitExists(pullRequest.lastHeadBuild.commit)) {
       log.debug(
         `Adding merged PR build commit ${pullRequest.lastHeadBuild.commit} to commits with builds`
       );
-      initialCommitsWithBuilds.push(pullRequest.lastHeadBuild.commit);
+      commitsWithBuilds.push(pullRequest.lastHeadBuild.commit);
+      // shouldRecurse = true;
     } else {
       log.debug(
         `Merged PR build commit ${pullRequest.lastHeadBuild.commit} not in index, blindly appending to parents`
@@ -250,24 +285,47 @@ export async function getParentCommits(
     }
   }
 
-  // Get a "covering" set of commits that have builds. This is a set of commits
-  // such that any ancestor of HEAD is either:
-  //   - in commitsWithBuilds
-  //   - an ancestor of a commit in commitsWithBuilds
-  //   - has no build
-  const commitsWithBuilds = await step(
-    { options, client, log, git },
-    FETCH_N_INITIAL_BUILD_COMMITS,
-    {
-      firstCommittedAtSeconds: firstBuild.committedAt && firstBuild.committedAt / 1000,
-      commitsWithBuilds: initialCommitsWithBuilds,
-      commitsWithoutBuilds: [],
-    }
-  );
+  // let finalCommitsWithBuilds = commitsWithBuilds;
+  // if (shouldRecurse) {
+  //   const { commitsWithBuilds: secondCommitsWithBuilds, visitedCommitsWithoutBuilds } = await step(
+  //     { options, client, log, git },
+  //     FETCH_N_INITIAL_BUILD_COMMITS,
+  //     {
+  //       firstCommittedAtSeconds: firstBuild.committedAt && firstBuild.committedAt / 1000,
+  //       commitsWithBuilds: commitsWithBuilds,
+  //       commitsWithoutBuilds,
+  //     }
+  //   );
+  //   finalCommitsWithBuilds = secondCommitsWithBuilds;
+  // }
+
+  // 1. Add new App.pullRequests(mergeInfo) resolver
+  // const extraPrs = runQuery(`app {
+  //       pullRequests(mergeInfo: $mergeInfos) {
+  //         lastHeadBuild {
+  //           commit
+  //         }
+  //       }
+  //     }`, { mergeInfos: mergeInfoFromTraversedCommits(traversedCommits) });
+  //
+
+  //   let shouldRecurse = false;
+  //   extraPrs.forEach(pr => {
+  //     if (await commitExists(pullRequest.lastCommit)) {
+  //       commitsWithBuilds.push(pullRequest.lastCommit);
+  //       shouldRecurse = true;
+  //     } else {
+  //       extraParentCommits.push(pullRequest.lastHeadBuild.commit);
+  //     }
+  //   });
+  //
+  // if (shouldRecurse) goto 258
 
   log.debug(`Final commitsWithBuilds: ${commitsWithBuilds}`);
 
   // For any pair A,B of builds, there is no point in using B if it is an ancestor of A.
   const descendentCommits = await maximallyDescendentCommits({ log }, commitsWithBuilds);
-  return extraParentCommits.concat(descendentCommits);
+
+  const ancestorCommits = extraParentCommits.concat(descendentCommits);
+  return ancestorCommits;
 }
diff --git a/node-src/git/mocks/mock-index.ts b/node-src/git/mocks/mock-index.ts
index c5692c33c..bf2c0e945 100644
--- a/node-src/git/mocks/mock-index.ts
+++ b/node-src/git/mocks/mock-index.ts
@@ -12,18 +12,19 @@
 
 // create a mock set of responses to the queries we run as part of our git algorithm
 
-const mocks = {
-  FirstCommittedAtQuery: (builds, prs, { branch, commit }) => {
-    function lastBuildOnBranch(findingBranch) {
-      return builds
-        .slice()
-        .reverse()
-        .find((b) => b.branch === findingBranch);
-    }
+type Build = { branch: string; commit: string; committedAt: number };
+type PR = { mergeCommitHash: string; headBranch: string };
 
-    const lastBuild = lastBuildOnBranch(branch);
-    const pr = prs.find((p) => p.mergeCommitHash === commit);
-    const prLastBuild = pr && lastBuildOnBranch(pr.headBranch);
+function lastBuildOnBranch(builds: Build[], findingBranch: string) {
+  return builds
+    .slice()
+    .reverse()
+    .find((b) => b.branch === findingBranch);
+}
+
+const mocks = {
+  FirstCommittedAtQuery: (builds: Build[], prs: PR[], { branch }: { branch: string }) => {
+    const lastBuild = lastBuildOnBranch(builds, branch);
     return {
       app: {
         firstBuild: builds[0] && {
@@ -33,6 +34,20 @@ const mocks = {
           commit: lastBuild.commit,
           committedAt: lastBuild.committedAt,
         },
+      },
+    };
+  },
+  HasBuildsWithCommitsQuery: (builds: Build[], prs: PR[], { commits }: { commits: string[] }) => ({
+    app: {
+      hasBuildsWithCommits: commits.filter((commit) => !!builds.find((b) => b.commit === commit)),
+    },
+  }),
+  IsMergeCommitQuery: (builds: Build[], prs: PR[], { commit }: { commit: string }) => {
+    const pr = prs.find((p) => p.mergeCommitHash === commit);
+    const prLastBuild = pr && lastBuildOnBranch(builds, pr.headBranch);
+
+    return {
+      app: {
         pullRequest: prLastBuild && {
           lastHeadBuild: {
             commit: prLastBuild.commit,
@@ -41,11 +56,6 @@ const mocks = {
       },
     };
   },
-  HasBuildsWithCommitsQuery: (builds, _prs, { commits }) => ({
-    app: {
-      hasBuildsWithCommits: commits.filter((commit) => !!builds.find((b) => b.commit === commit)),
-    },
-  }),
 };
 
 export default function createMockIndex({ commitMap }, buildDescriptions, prDescriptions = []) {

From 95348650aa88057dff1dbffb6b1cfc2448d4cd77 Mon Sep 17 00:00:00 2001
From: Tom Coleman <tom@chromatic.com>
Date: Tue, 28 Nov 2023 12:38:49 +1100
Subject: [PATCH 02/11] Update code to check all visited commits

---
 node-src/git/getParentCommits.test.ts | 12 ++++
 node-src/git/getParentCommits.ts      | 96 +++++++++------------------
 2 files changed, 44 insertions(+), 64 deletions(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index 624d80c8c..ab8bbb659 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -687,10 +687,22 @@ describe('getParentCommits', () => {
 
     // it(`deals with situations where squashed branches have no builds`)
 
+    //       [X]
+    //     /    \
+    //    P - Q - R
+    //  /
+    //[A] - B - C - M
+
     // it(`deals with situations where squashed branches no longer exist in the repo but have a build`)
 
     // it(`deals with situations where squashed branches no longer exist in the repo and have no build`)
 
     // it(`deals with situations where squashed branches themselves have squash merge commits`)
+
+    // P -[Q]
+    //  \
+    //   X - [Y] -<Z>   (squash merge of Q)
+    //    \
+    //      A -    -   <M> (squash merge of Z)
   });
 });
diff --git a/node-src/git/getParentCommits.ts b/node-src/git/getParentCommits.ts
index b32479cc9..d83aa8540 100644
--- a/node-src/git/getParentCommits.ts
+++ b/node-src/git/getParentCommits.ts
@@ -154,7 +154,7 @@ async function step(
   // No more commits uncovered commitsWithBuilds!
   if (candidateCommits.length === 0) {
     log.debug('step: no candidateCommits; we are done');
-    return { commitsWithBuilds, commitsWithoutBuilds, visitedCommitsWithoutBuilds };
+    return { commitsWithBuilds, visitedCommitsWithoutBuilds };
   }
 
   const {
@@ -199,7 +199,7 @@ export async function getParentCommits(
   { options, client, git, log }: Context,
   { ignoreLastBuildOnBranch = false } = {}
 ) {
-  const { branch, commit, committedAt } = git;
+  const { branch, committedAt } = git;
 
   // Include the latest build from this branch as an ancestor of the current build
   const { app } = await client.runQuery<FirstCommittedAtQueryResult>(
@@ -247,7 +247,7 @@ export async function getParentCommits(
   //   - in commitsWithBuilds
   //   - an ancestor of a commit in commitsWithBuilds
   //   - has no build
-  const { commitsWithBuilds } = await step(
+  const { commitsWithBuilds, visitedCommitsWithoutBuilds } = await step(
     { options, client, log, git },
     FETCH_N_INITIAL_BUILD_COMMITS,
     {
@@ -257,69 +257,37 @@ export async function getParentCommits(
     }
   );
 
-  // Check if the current commit is a merge commit
-  const {
-    app: { pullRequest },
-  } = await client.runQuery<IsMergeCommitQueryResult>(
-    IsMergeCommitQuery,
-    { commit, branch },
-    { retries: 5 } // This query requires a request to an upstream provider which may fail
-  );
-
-  // Add the most recent build on a (merged) branch as a parent if we think this was the commit that
-  // merged the pull request.
-  // @see https://www.chromatic.com/docs/branching-and-baselines#squash-and-rebase-merging
-  // let shouldRecurse = false;
-  if (pullRequest && pullRequest.lastHeadBuild) {
-    if (await commitExists(pullRequest.lastHeadBuild.commit)) {
-      log.debug(
-        `Adding merged PR build commit ${pullRequest.lastHeadBuild.commit} to commits with builds`
+  // TODO: what if visitedCommitsWithoutBuilds is really long?
+
+  await Promise.all(
+    visitedCommitsWithoutBuilds.map(async (commit) => {
+      // Check if the visited commit is a merge commit
+      const {
+        app: { pullRequest },
+      } = await client.runQuery<IsMergeCommitQueryResult>(
+        IsMergeCommitQuery,
+        { commit, branch },
+        { retries: 5 } // This query requires a request to an upstream provider which may fail
       );
-      commitsWithBuilds.push(pullRequest.lastHeadBuild.commit);
-      // shouldRecurse = true;
-    } else {
-      log.debug(
-        `Merged PR build commit ${pullRequest.lastHeadBuild.commit} not in index, blindly appending to parents`
-      );
-      extraParentCommits.push(pullRequest.lastHeadBuild.commit);
-    }
-  }
 
-  // let finalCommitsWithBuilds = commitsWithBuilds;
-  // if (shouldRecurse) {
-  //   const { commitsWithBuilds: secondCommitsWithBuilds, visitedCommitsWithoutBuilds } = await step(
-  //     { options, client, log, git },
-  //     FETCH_N_INITIAL_BUILD_COMMITS,
-  //     {
-  //       firstCommittedAtSeconds: firstBuild.committedAt && firstBuild.committedAt / 1000,
-  //       commitsWithBuilds: commitsWithBuilds,
-  //       commitsWithoutBuilds,
-  //     }
-  //   );
-  //   finalCommitsWithBuilds = secondCommitsWithBuilds;
-  // }
-
-  // 1. Add new App.pullRequests(mergeInfo) resolver
-  // const extraPrs = runQuery(`app {
-  //       pullRequests(mergeInfo: $mergeInfos) {
-  //         lastHeadBuild {
-  //           commit
-  //         }
-  //       }
-  //     }`, { mergeInfos: mergeInfoFromTraversedCommits(traversedCommits) });
-  //
-
-  //   let shouldRecurse = false;
-  //   extraPrs.forEach(pr => {
-  //     if (await commitExists(pullRequest.lastCommit)) {
-  //       commitsWithBuilds.push(pullRequest.lastCommit);
-  //       shouldRecurse = true;
-  //     } else {
-  //       extraParentCommits.push(pullRequest.lastHeadBuild.commit);
-  //     }
-  //   });
-  //
-  // if (shouldRecurse) goto 258
+      // Add the most recent build on a (merged) branch as an ancestor if we visit a commit
+      // during our ancestor selection that was the merge commit for that PR.
+      // @see https://www.chromatic.com/docs/branching-and-baselines#squash-and-rebase-merging
+      if (pullRequest && pullRequest.lastHeadBuild) {
+        if (await commitExists(pullRequest.lastHeadBuild.commit)) {
+          log.debug(
+            `Adding merged PR build commit ${pullRequest.lastHeadBuild.commit} to commits with builds`
+          );
+          commitsWithBuilds.push(pullRequest.lastHeadBuild.commit);
+        } else {
+          log.debug(
+            `Merged PR build commit ${pullRequest.lastHeadBuild.commit} not in index, blindly appending to parents`
+          );
+          extraParentCommits.push(pullRequest.lastHeadBuild.commit);
+        }
+      }
+    })
+  );
 
   log.debug(`Final commitsWithBuilds: ${commitsWithBuilds}`);
 

From b6e57720f4c974f09fa2fc7a9598724583b0ece5 Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Thu, 30 Nov 2023 12:23:06 -0800
Subject: [PATCH 03/11] use new query for looking up multiple merged pull
 requests

---
 node-src/git/getParentCommits.ts | 67 +++++++++++++++-----------------
 node-src/git/mocks/mock-index.ts | 23 ++++++-----
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/node-src/git/getParentCommits.ts b/node-src/git/getParentCommits.ts
index d83aa8540..d6f755c73 100644
--- a/node-src/git/getParentCommits.ts
+++ b/node-src/git/getParentCommits.ts
@@ -45,10 +45,10 @@ interface HasBuildsWithCommitsQueryResult {
   };
 }
 
-const IsMergeCommitQuery = gql`
-  query IsMergeCommitQuery($commit: String!, $branch: String!) {
+const MergeCommitsQuery = gql`
+  query MergeCommitsQuery($mergeInfos: [MergedInfoInput]!) {
     app {
-      pullRequest(mergeInfo: { commit: $commit, baseRefName: $branch }) {
+      mergedPullRequests(mergeInfos: $mergeInfos) {
         lastHeadBuild {
           commit
         }
@@ -56,13 +56,13 @@ const IsMergeCommitQuery = gql`
     }
   }
 `;
-interface IsMergeCommitQueryResult {
+interface MergeCommitsQueryResult {
   app: {
-    pullRequest: {
+    mergedPullRequests: [{
       lastHeadBuild: {
         commit: string;
       };
-    };
+    }];
   };
 }
 
@@ -257,37 +257,34 @@ export async function getParentCommits(
     }
   );
 
-  // TODO: what if visitedCommitsWithoutBuilds is really long?
-
-  await Promise.all(
-    visitedCommitsWithoutBuilds.map(async (commit) => {
-      // Check if the visited commit is a merge commit
-      const {
-        app: { pullRequest },
-      } = await client.runQuery<IsMergeCommitQueryResult>(
-        IsMergeCommitQuery,
-        { commit, branch },
-        { retries: 5 } // This query requires a request to an upstream provider which may fail
-      );
+  const mergeInfos = visitedCommitsWithoutBuilds.map((commit) => {
+    return { commit, baseRefName: branch };
+  });
+  const { app: { mergedPullRequests } } = await client.runQuery<MergeCommitsQueryResult>(
+    MergeCommitsQuery,
+    { mergeInfos },
+    { retries: 5 } // This query requires a request to an upstream provider which may fail
+  );
 
-      // Add the most recent build on a (merged) branch as an ancestor if we visit a commit
-      // during our ancestor selection that was the merge commit for that PR.
-      // @see https://www.chromatic.com/docs/branching-and-baselines#squash-and-rebase-merging
-      if (pullRequest && pullRequest.lastHeadBuild) {
-        if (await commitExists(pullRequest.lastHeadBuild.commit)) {
-          log.debug(
-            `Adding merged PR build commit ${pullRequest.lastHeadBuild.commit} to commits with builds`
-          );
-          commitsWithBuilds.push(pullRequest.lastHeadBuild.commit);
-        } else {
-          log.debug(
-            `Merged PR build commit ${pullRequest.lastHeadBuild.commit} not in index, blindly appending to parents`
-          );
-          extraParentCommits.push(pullRequest.lastHeadBuild.commit);
-        }
+  for (const pullRequest of mergedPullRequests) {
+    // Add the most recent build on a (merged) branch as an ancestor if we visit a commit
+    // during our ancestor selection that was the merge commit for that PR.
+    // @see https://www.chromatic.com/docs/branching-and-baselines#squash-and-rebase-merging
+    const lastHeadBuildCommit = pullRequest.lastHeadBuild?.commit;
+    if (lastHeadBuildCommit) {
+      if (await commitExists(lastHeadBuildCommit)) {
+        log.debug(
+          `Adding merged PR build commit ${lastHeadBuildCommit} to commits with builds`
+        );
+        commitsWithBuilds.push(lastHeadBuildCommit);
+      } else {
+        log.debug(
+          `Merged PR build commit ${lastHeadBuildCommit} not in index, blindly appending to parents`
+        );
+        extraParentCommits.push(lastHeadBuildCommit);
       }
-    })
-  );
+    }
+  }
 
   log.debug(`Final commitsWithBuilds: ${commitsWithBuilds}`);
 
diff --git a/node-src/git/mocks/mock-index.ts b/node-src/git/mocks/mock-index.ts
index bf2c0e945..90fa54b82 100644
--- a/node-src/git/mocks/mock-index.ts
+++ b/node-src/git/mocks/mock-index.ts
@@ -14,6 +14,7 @@
 
 type Build = { branch: string; commit: string; committedAt: number };
 type PR = { mergeCommitHash: string; headBranch: string };
+type MergeInfo = { commit: string; branch: string; }
 
 function lastBuildOnBranch(builds: Build[], findingBranch: string) {
   return builds
@@ -42,18 +43,22 @@ const mocks = {
       hasBuildsWithCommits: commits.filter((commit) => !!builds.find((b) => b.commit === commit)),
     },
   }),
-  IsMergeCommitQuery: (builds: Build[], prs: PR[], { commit }: { commit: string }) => {
-    const pr = prs.find((p) => p.mergeCommitHash === commit);
-    const prLastBuild = pr && lastBuildOnBranch(builds, pr.headBranch);
+  MergeCommitsQuery: (builds: Build[], prs: PR[], { mergeInfos }: { mergeInfos: MergeInfo[] }) => {
+    const mergedPrs = [];
+    for (const mergeInfo of mergeInfos) {
+      const pr = prs.find((p) => p.mergeCommitHash === mergeInfo.commit);
+      const prLastBuild = pr && lastBuildOnBranch(builds, pr.headBranch);
+      mergedPrs.push({
+        lastHeadBuild: prLastBuild && {
+          commit: prLastBuild.commit,
+        }
+      });
+    }
 
     return {
       app: {
-        pullRequest: prLastBuild && {
-          lastHeadBuild: {
-            commit: prLastBuild.commit,
-          },
-        },
-      },
+        mergedPullRequests: mergedPrs
+      }
     };
   },
 };

From bbdf7d9820a4e45ff68d9a12464eb401a6961a77 Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Thu, 30 Nov 2023 16:47:40 -0800
Subject: [PATCH 04/11] rename

---
 node-src/git/getParentCommits.ts | 8 ++++----
 node-src/git/mocks/mock-index.ts | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/node-src/git/getParentCommits.ts b/node-src/git/getParentCommits.ts
index d6f755c73..56d1307ea 100644
--- a/node-src/git/getParentCommits.ts
+++ b/node-src/git/getParentCommits.ts
@@ -46,9 +46,9 @@ interface HasBuildsWithCommitsQueryResult {
 }
 
 const MergeCommitsQuery = gql`
-  query MergeCommitsQuery($mergeInfos: [MergedInfoInput]!) {
+  query MergeCommitsQuery($mergeInfoList: [MergedInfoInput]!) {
     app {
-      mergedPullRequests(mergeInfos: $mergeInfos) {
+      mergedPullRequests(mergeInfoList: $mergeInfoList) {
         lastHeadBuild {
           commit
         }
@@ -257,12 +257,12 @@ export async function getParentCommits(
     }
   );
 
-  const mergeInfos = visitedCommitsWithoutBuilds.map((commit) => {
+  const mergeInfoList = visitedCommitsWithoutBuilds.map((commit) => {
     return { commit, baseRefName: branch };
   });
   const { app: { mergedPullRequests } } = await client.runQuery<MergeCommitsQueryResult>(
     MergeCommitsQuery,
-    { mergeInfos },
+    { mergeInfoList },
     { retries: 5 } // This query requires a request to an upstream provider which may fail
   );
 
diff --git a/node-src/git/mocks/mock-index.ts b/node-src/git/mocks/mock-index.ts
index 90fa54b82..56e3a179b 100644
--- a/node-src/git/mocks/mock-index.ts
+++ b/node-src/git/mocks/mock-index.ts
@@ -43,9 +43,9 @@ const mocks = {
       hasBuildsWithCommits: commits.filter((commit) => !!builds.find((b) => b.commit === commit)),
     },
   }),
-  MergeCommitsQuery: (builds: Build[], prs: PR[], { mergeInfos }: { mergeInfos: MergeInfo[] }) => {
+  MergeCommitsQuery: (builds: Build[], prs: PR[], { mergeInfoList }: { mergeInfoList: MergeInfo[] }) => {
     const mergedPrs = [];
-    for (const mergeInfo of mergeInfos) {
+    for (const mergeInfo of mergeInfoList) {
       const pr = prs.find((p) => p.mergeCommitHash === mergeInfo.commit);
       const prLastBuild = pr && lastBuildOnBranch(builds, pr.headBranch);
       mergedPrs.push({

From 7bc27a930cc2a374cfd760af1f3f29203ecdddd3 Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Thu, 30 Nov 2023 16:51:34 -0800
Subject: [PATCH 05/11] limit amount sent in API request

---
 node-src/git/getParentCommits.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/node-src/git/getParentCommits.ts b/node-src/git/getParentCommits.ts
index 56d1307ea..343056164 100644
--- a/node-src/git/getParentCommits.ts
+++ b/node-src/git/getParentCommits.ts
@@ -262,7 +262,7 @@ export async function getParentCommits(
   });
   const { app: { mergedPullRequests } } = await client.runQuery<MergeCommitsQueryResult>(
     MergeCommitsQuery,
-    { mergeInfoList },
+    { mergeInfoList: mergeInfoList.slice(0, 100) }, // Limit amount sent in API call
     { retries: 5 } // This query requires a request to an upstream provider which may fail
   );
 

From 194e9a7065f9eccd54b55d4e432eb60690694ace Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Fri, 1 Dec 2023 14:09:43 -0800
Subject: [PATCH 06/11] add some tests

---
 node-src/git/getParentCommits.test.ts | 91 +++++++++++++++++++++------
 node-src/git/mocks/double-loop.ts     | 23 +++++++
 2 files changed, 95 insertions(+), 19 deletions(-)
 create mode 100644 node-src/git/mocks/double-loop.ts

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index ab8bbb659..2fcc3c72e 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -10,6 +10,7 @@ import generateGitRepository from './generateGitRepository';
 
 import longLineDescription from './mocks/long-line';
 import longLoopDescription from './mocks/long-loop';
+import doubleLoopDescription from './mocks/double-loop';
 import createMockIndex from './mocks/mock-index';
 import simpleLoopDescription from './mocks/simple-loop';
 import threeParentsDescription from './mocks/three-parents';
@@ -19,6 +20,7 @@ const descriptions = {
   simpleLoop: simpleLoopDescription,
   longLine: longLineDescription,
   longLoop: longLoopDescription,
+  doubleLoop: doubleLoopDescription,
   threeParents: threeParentsDescription,
   twoRoots: twoRootsDescription,
 };
@@ -615,7 +617,7 @@ describe('getParentCommits', () => {
     it(`also includes PR head commits that were squashed to this commit`, async () => {
       //
       //
-      //             [D] [branch, squash merged into E]
+      //             [D]   [branch, squash merged into E]
       //            /   \
       //  A - B -[C]      F
       //            \   /
@@ -641,9 +643,9 @@ describe('getParentCommits', () => {
     it(`also finds squash merge commits that were on previous commits without builds`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //
-      //     B -[D]     [branch]
+      //     B -[D]      [branch]
       //    /
-      //  A - [C]-<E>-(G) [main]
+      //  A -[C]-<E>-(G) [main]
       const repository = repositories.longLoop;
       await checkoutCommit('G', 'main', repository);
       const client = createClient({
@@ -664,7 +666,7 @@ describe('getParentCommits', () => {
     it(`deals with situations where the last build on the squashed branch isn't the last commit`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //
-      //     [B] - D      [branch]
+      //     [B]- D      [branch]
       //    /
       //  A -[C]-<E>-(G) [main]
       const repository = repositories.longLoop;
@@ -681,28 +683,79 @@ describe('getParentCommits', () => {
       const git = { branch: 'main', ...(await getCommit()) };
 
       const parentCommits = await getParentCommits({ client, log, git, options } as any);
-      // This doesn't include 'C' as D "covers" it.
       expectCommitsToEqualNames(parentCommits, ['C', 'B'], repository);
     });
 
-    // it(`deals with situations where squashed branches have no builds`)
+    it(`deals with situations where squashed branches have no builds`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //     B - D       [branch]
+      //    /
+      //  A -[C]-<E>-(G) [main]
+      const repository = repositories.longLoop;
+      await checkoutCommit('G', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['C', 'main'],
+        ],
+        // Talking to GH (etc) tells us that commit E is the merge commit for "branch"
+        prs: [['E', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
 
-    //       [X]
-    //     /    \
-    //    P - Q - R
-    //  /
-    //[A] - B - C - M
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      expectCommitsToEqualNames(parentCommits, ['C'], repository);
+    });
 
-    // it(`deals with situations where squashed branches no longer exist in the repo but have a build`)
+    it(`deals with situations where squashed branches no longer exist in the repo but have a build`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //  [B] [no longer in repo]
+      //
+      //  [A]- C -<(D)> [main]
+      const repository = repositories.twoRoots;
+      await checkoutCommit('D', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['A', 'main'],
+          ['B', 'xxx'],
+        ],
+        // Talking to GH (etc) tells us that commit D is the merge commit for "branch" (which no longer exists)
+        prs: [['D', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
 
-    // it(`deals with situations where squashed branches no longer exist in the repo and have no build`)
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      // This is A and not B because we do not know anything about the deleted branch and its commits
+      expectCommitsToEqualNames(parentCommits, ['A'], repository);
+    });
 
-    // it(`deals with situations where squashed branches themselves have squash merge commits`)
+    it(`deals with situations where squashed branches themselves have squash merge commits`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //       [F]        [branch2]
+      //      /
+      //    [C]-<E>       [branch]
+      //   /
+      //  A -[B]-<D>- G  [main]
+      const repository = repositories.doubleLoop;
+      await checkoutCommit('G', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['B', 'main'],
+          ['C', 'branch'],
+          ['F', 'branch2'],
+        ],
+        // Talking to GH (etc) tells us that commit G is the merge commit for "branch"
+        prs: [['D', 'branch'], ['E', 'branch2']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
 
-    // P -[Q]
-    //  \
-    //   X - [Y] -<Z>   (squash merge of Q)
-    //    \
-    //      A -    -   <M> (squash merge of Z)
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      expectCommitsToEqualNames(parentCommits, ['C', 'B'], repository);
+    });
   });
 });
diff --git a/node-src/git/mocks/double-loop.ts b/node-src/git/mocks/double-loop.ts
new file mode 100644
index 000000000..5514f7f31
--- /dev/null
+++ b/node-src/git/mocks/double-loop.ts
@@ -0,0 +1,23 @@
+// A
+// | \
+// B  C
+// |  | \
+// D  E  F
+// |  | /
+// G  H
+// | /
+// I
+
+// [commit, parent(s)]
+export default [
+  // prettier-ignore
+  ['A', false],
+  ['B', 'A'],
+  ['C', 'A'],
+  ['D', 'B'],
+  ['E', 'C'],
+  ['F', 'C'],
+  ['G', 'D'],
+  ['H', 'E'],
+  ['I', ['G', 'H']],
+];

From eb76c00f412e07f816c88a36db84168ac89f3ac4 Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Mon, 4 Dec 2023 15:54:00 -0800
Subject: [PATCH 07/11] fix name

---
 node-src/git/getParentCommits.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index 2fcc3c72e..023e562ec 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -732,7 +732,7 @@ describe('getParentCommits', () => {
       expectCommitsToEqualNames(parentCommits, ['A'], repository);
     });
 
-    it(`deals with situations where squashed branches themselves have squash merge commits`, async () => {
+    it(`does not find parents for commits on a squashed branch that are themselves squash merge commits`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //
       //       [F]        [branch2]

From ac447d06cfcdf14e7dd982b7a9d20cae94256296 Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Mon, 4 Dec 2023 16:00:00 -0800
Subject: [PATCH 08/11] add new test

---
 node-src/git/getParentCommits.test.ts | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index 023e562ec..e2ade9e08 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -686,6 +686,30 @@ describe('getParentCommits', () => {
       expectCommitsToEqualNames(parentCommits, ['C', 'B'], repository);
     });
 
+    it(`occludes commits that have a more recent build on a squashed branch`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //
+      //     [B]- D      [branch]
+      //    /
+      //  [A]- C -<E>-(G) [main]
+      const repository = repositories.longLoop;
+      await checkoutCommit('G', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['A', 'main'],
+          ['B', 'branch'],
+        ],
+        // Talking to GH (etc) tells us that commit E is the merge commit for "branch"
+        prs: [['E', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
+
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      // This doesn't include A as B "covers" it.
+      expectCommitsToEqualNames(parentCommits, ['B'], repository);
+    });
+
     it(`deals with situations where squashed branches have no builds`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //

From 84f11f8f2b843b9a7c1e9f232593d204a2438d0d Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Tue, 5 Dec 2023 09:53:55 -0800
Subject: [PATCH 09/11] allow for commits that no longer exist in the history

---
 node-src/git/getParentCommits.test.ts |  2 +-
 node-src/git/mocks/mock-index.ts      | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index e2ade9e08..d4b1a8c87 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -51,7 +51,7 @@ function createClient({
 }
 
 function expectCommitsToEqualNames(hashes, names, { commitMap }) {
-  return expect(hashes).toEqual(names.map((n) => commitMap[n].hash));
+  return expect(hashes).toEqual(names.map((name) => commitMap[name]?.hash || name));
 }
 
 async function checkoutCommit(name, branch, { dirname, runGit, commitMap }) {
diff --git a/node-src/git/mocks/mock-index.ts b/node-src/git/mocks/mock-index.ts
index 56e3a179b..18a078c34 100644
--- a/node-src/git/mocks/mock-index.ts
+++ b/node-src/git/mocks/mock-index.ts
@@ -65,8 +65,16 @@ const mocks = {
 
 export default function createMockIndex({ commitMap }, buildDescriptions, prDescriptions = []) {
   const builds = buildDescriptions.map(([name, branch], index) => {
-    const { hash, committedAt: committedAtSeconds } = commitMap[name];
-    const committedAt = parseInt(committedAtSeconds, 10) * 1000;
+    let hash, committedAt;
+    if (commitMap[name]) {
+      const commitInfo = commitMap[name];
+      hash = commitInfo.hash;
+      committedAt = parseInt(commitInfo.committedAt, 10) * 1000;
+    } else {
+      // Allow for test cases with a commit that is no longer in the history
+      hash = name;
+      committedAt = Date.now();
+    }
 
     const number = index + 1;
     return {

From b9cf5226ff01ec00275a83ddb2f36f997a206a6d Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Tue, 5 Dec 2023 09:54:08 -0800
Subject: [PATCH 10/11] tweak tests

---
 node-src/git/getParentCommits.test.ts | 33 +++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index d4b1a8c87..1bc81f9e6 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -135,7 +135,7 @@ describe('getParentCommits', () => {
       repository,
       builds: [
         ['D', 'main'],
-        ['E', 'main'],
+        ['E', 'branch'],
       ],
     });
     const git = { branch: 'main', ...(await getCommit()) };
@@ -735,7 +735,7 @@ describe('getParentCommits', () => {
     it(`deals with situations where squashed branches no longer exist in the repo but have a build`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //
-      //  [B] [no longer in repo]
+      //  [MISSING] [no longer in repo]
       //
       //  [A]- C -<(D)> [main]
       const repository = repositories.twoRoots;
@@ -744,7 +744,7 @@ describe('getParentCommits', () => {
         repository,
         builds: [
           ['A', 'main'],
-          ['B', 'xxx'],
+          ['MISSING', 'branch'],
         ],
         // Talking to GH (etc) tells us that commit D is the merge commit for "branch" (which no longer exists)
         prs: [['D', 'branch']],
@@ -753,9 +753,12 @@ describe('getParentCommits', () => {
 
       const parentCommits = await getParentCommits({ client, log, git, options } as any);
       // This is A and not B because we do not know anything about the deleted branch and its commits
-      expectCommitsToEqualNames(parentCommits, ['A'], repository);
+      expectCommitsToEqualNames(parentCommits, ['MISSING', 'A'], repository);
     });
 
+    // We could support this situation via recursing and doing a second (etc) check for squash merge commits,
+    // but that would make things a lot more complex and this situation seems quite unlikely and is easily avoided
+    // by ensuring you run a build for E.
     it(`does not find parents for commits on a squashed branch that are themselves squash merge commits`, async () => {
       // []: has build, <>: is squash merge, (): current commit
       //
@@ -781,5 +784,27 @@ describe('getParentCommits', () => {
       const parentCommits = await getParentCommits({ client, log, git, options } as any);
       expectCommitsToEqualNames(parentCommits, ['C', 'B'], repository);
     });
+
+    it(`does not affect finding a merge commit's parents (in correct order) when they already have a build`, async () => {
+      // []: has build, <>: is squash merge, (): current commit
+      //  A - B - C -[D]-(F)  [main]
+      //            \   /
+      //             [E]      [branch]
+      const repository = repositories.simpleLoop;
+      await checkoutCommit('F', 'main', repository);
+      const client = createClient({
+        repository,
+        builds: [
+          ['D', 'main'],
+          ['E', 'branch'],
+        ],
+        // Talking to GH (etc) tells us that commit F is the merge commit for "branch"
+        prs: [['F', 'branch']],
+      });
+      const git = { branch: 'main', ...(await getCommit()) };
+  
+      const parentCommits = await getParentCommits({ client, log, git, options } as any);
+      expectCommitsToEqualNames(parentCommits, ['E', 'D'], repository);
+    });
   });
 });

From 026df68b4429a8b7e8813beaa8eaa1475caa026e Mon Sep 17 00:00:00 2001
From: Todd Evanoff <evanoff@gmail.com>
Date: Tue, 5 Dec 2023 15:13:40 -0800
Subject: [PATCH 11/11] remove erroneous comment

---
 node-src/git/getParentCommits.test.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/node-src/git/getParentCommits.test.ts b/node-src/git/getParentCommits.test.ts
index 1bc81f9e6..cb476c4ce 100644
--- a/node-src/git/getParentCommits.test.ts
+++ b/node-src/git/getParentCommits.test.ts
@@ -752,7 +752,6 @@ describe('getParentCommits', () => {
       const git = { branch: 'main', ...(await getCommit()) };
 
       const parentCommits = await getParentCommits({ client, log, git, options } as any);
-      // This is A and not B because we do not know anything about the deleted branch and its commits
       expectCommitsToEqualNames(parentCommits, ['MISSING', 'A'], repository);
     });