fix(changelog): Credit PR commit co-authors when the merge commit has none

docs/src/content/docs/changelogs.md

@@ -288,7 +288,8 @@ changelog:
 
 Entries credit the PR author and any co-authors from `Co-authored-by` commit
 trailers that GitHub links to an account, for example `by @alice, @bob and 2
-others`. Bots are credited after people, as in `by @alice with @some-bot`. To
+others`. When the merged commit has no co-authors, for example because a squash
+merge cleared its message, Craft reads them from the commits of the PR. Bots are credited after people, as in `by @alice with @some-bot`. To
 keep a PR in the changelog without crediting an author, list it in
 `hide_authors` in `.github/release.yml`:
 

src/utils/__tests__/changelog-generate.test.ts

@@ -912,6 +912,72 @@ describe('generateChangesetFromGit', () => {
       );
     });
 
+    it('credits PR commit co-authors when the merged commit has none', async () => {
+      const commit = junior([]);
+      // GraphQL returns PR numbers as numbers
+      commit.pr!.remote!.number = 123 as unknown as string;
+      setup([commit], null);
+      mockClient.mockResolvedValueOnce({
+        repository: {
+          P123: {
+            commits: {
+              nodes: [
+                {
+                  commit: {
+                    authors: commitAuthors(['sentry-junior[bot]', 'BYK']),
+                  },
+                },
+                { commit: { authors: commitAuthors(['thomasjiangcy']) } },
+              ],
+            },
+          },
+        },
+      });
+      const result = await generateChangesetFromGit(dummyGit, '1.0.0', 3);
+      expect(result.changelog).toContain(
+        'Upgraded the kernel by @BYK and @thomasjiangcy with @sentry-junior in [#123]',
+      );
+    });
+
+    it('keeps PR co-authors that load when another PR number fails', async () => {
+      setup(
+        [
+          junior([]),
+          {
+            hash: 'bcdef1234567890a',
+            title: 'Fixed the kernel',
+            body: '',
+            pr: { local: '999' },
+          },
+        ],
+        null,
+      );
+      mockClient.mockRejectedValueOnce(
+        Object.assign(new Error('Could not resolve to a PullRequest'), {
+          data: {
+            repository: {
+              P123: {
+                commits: {
+                  nodes: [{ commit: { authors: commitAuthors(['BYK']) } }],
+                },
+              },
+              P999: null,
+            },
+          },
+        }),
+      );
+      const result = await generateChangesetFromGit(dummyGit, '1.0.0', 3);
+      expect(result.changelog).toContain(
+        'Upgraded the kernel by @BYK with @sentry-junior in [#123]',
+      );
+    });
+
+    it('does not read the PR when the merged commit has co-authors', async () => {
+      setup([junior(['jan-auer'])], null);
+      await generateChangesetFromGit(dummyGit, '1.0.0', 3);
+      expect(mockClient).toHaveBeenCalledTimes(1);
+    });
+
     it('collapses long author lists', async () => {
       setup(
         [
@@ -2444,7 +2510,7 @@ describe('generateChangelogWithHighlight', () => {
     });
     mockGraphqlClient.mockResolvedValueOnce({
       repository: {
-        pullRequest: {
+        P7: {
           commits: {
             nodes: [
               { commit: { authors: commitAuthors(['bob']) } },

src/utils/changelog.ts

@@ -107,6 +107,14 @@ interface CommitAuthors {
   nodes: Array<{ user: { login: string } | null }>;
 }
 
+/** PRs keyed by `P<number>` aliases, `null` for numbers that are not PRs */
+interface PRCommitAuthorsResult {
+  repository?: Record<
+    string,
+    { commits: { nodes: Array<{ commit: { authors: CommitAuthors } }> } } | null
+  >;
+}
+
 /** Returns the GitHub logins of commit authors that GitHub matched to a user. */
 function authorLogins(nodes: CommitAuthors['nodes'] = []): string[] {
   return nodes
@@ -116,47 +124,61 @@ function authorLogins(nodes: CommitAuthors['nodes'] = []): string[] {
 
 /**
  * Fetches the GitHub logins of everyone who authored or co-authored a commit
- * in the given PR. Co-authors come from `Co-authored-by` commit trailers.
+ * in the given PRs. Co-authors come from `Co-authored-by` commit trailers.
  *
- * Co-authors only add credit, so failures return an empty list.
+ * Co-authors only add credit, so PRs that fail to load are left out.
  *
- * @param prNumber The PR number to fetch
- * @returns Logins in commit order, possibly with duplicates
+ * @param prNumbers The PR numbers to fetch
+ * @returns Logins per PR number in commit order, possibly with duplicates
  */
-async function fetchPRCoAuthors(prNumber: number): Promise<string[]> {
+async function fetchPRCoAuthors(
+  prNumbers: string[],
+): Promise<Record<string, string[]>> {
   const { repo, owner } = await getGlobalGitHubConfig();
-  try {
-    const response = (await getGitHubClient().graphql(
-      `query($owner: String!, $repo: String!, $number: Int!) {
-        repository(owner: $owner, name: $repo) {
-          pullRequest(number: $number) {
+  const coAuthors: Record<string, string[]> = {};
+  for (let start = 0; start < prNumbers.length; start += MAX_PRS_PER_QUERY) {
+    const subset = prNumbers.slice(start, start + MAX_PRS_PER_QUERY);
+    const prsQuery = subset
+      // Aliases cannot start with a number, so prefix them with `P`
+      .map(
+        number => `P${number}: pullRequest(number: ${Number(number)}) {
             commits(first: ${MAX_PR_COMMITS}) {
               nodes {
                 commit {
                   authors(first: ${MAX_COMMIT_AUTHORS}) { nodes { user { login } } }
                 }
               }
             }
+          }`,
+      )
+      .join('\n');
+    let response: PRCommitAuthorsResult | undefined;
+    try {
+      response = (await getGitHubClient().graphql(
+        `query($owner: String!, $repo: String!) {
+          repository(owner: $owner, name: $repo) {
+            ${prsQuery}
           }
-        }
-      }`,
-      { owner, repo, number: prNumber },
-    )) as {
-      repository?: {
-        pullRequest?: {
-          commits: { nodes: Array<{ commit: { authors: CommitAuthors } }> };
-        };
-      };
-    };
-    return (response?.repository?.pullRequest?.commits.nodes ?? []).flatMap(
-      node => authorLogins(node.commit.authors.nodes),
-    );
-  } catch (error: any) {
-    logger.warn(
-      `Failed to fetch co-authors for PR #${prNumber}: ${error.message || error}`,
-    );
-    return [];
+        }`,
+        { owner, repo },
+      )) as PRCommitAuthorsResult;
+    } catch (error: any) {
+      logger.warn(
+        `Failed to fetch co-authors for PRs ${subset.map(n => `#${n}`).join(', ')}: ${error.message || error}`,
+      );
+      // A number that is not a PR fails the query but keeps the other PRs
+      response = error.data;
+    }
+    for (const number of subset) {
+      const pr = response?.repository?.[`P${number}`];
+      if (pr) {
+        coAuthors[number] = pr.commits.nodes.flatMap(node =>
+          authorLogins(node.commit.authors.nodes),
+        );
+      }
+    }
   }
+  return coAuthors;
 }
 
 /**
@@ -193,8 +215,10 @@ const VERSION_HEADER_LEVEL = 2;
 const SUBSECTION_HEADER_LEVEL = VERSION_HEADER_LEVEL + 1;
 const SCOPE_HEADER_LEVEL = SUBSECTION_HEADER_LEVEL + 1;
 const MAX_COMMITS_PER_QUERY = 50;
-/** Commits read from the current PR when previewing its changelog entry */
+/** Commits read from a PR to find its co-authors */
 const MAX_PR_COMMITS = 100;
+/** PRs per co-author query; each reads up to {@link MAX_PR_COMMITS} commits */
+const MAX_PRS_PER_QUERY = 25;
 /** Authors read per commit: the git author plus `Co-authored-by` trailers */
 const MAX_COMMIT_AUTHORS = 10;
 /** Authors shown per changelog entry before collapsing into "N others" */
@@ -1829,7 +1853,7 @@ export async function generateChangelogWithHighlight(
       body: prInfo.body,
       author: prInfo.author,
       authorIsBot: prInfo.authorIsBot,
-      coAuthors: await fetchPRCoAuthors(currentPRNumber),
+      coAuthors: (await fetchPRCoAuthors([currentPRStr]))[currentPRStr],
       pr: currentPRStr,
       prTitle: prInfo.title,
       prBody: prInfo.body,
@@ -1953,6 +1977,20 @@ async function fetchRawCommitInfo(
     result.push(commit);
   }
 
+  // A squash merge can drop the `Co-authored-by` trailers of the PR commits,
+  // for example when the merge message is cleared. Read them from the PR.
+  // GraphQL returns PR numbers as numbers despite the `string` type, so
+  // compare them as strings.
+  const withoutCoAuthors = [...seenPRs.values()].filter(
+    commit => !commit.coAuthors?.length,
+  );
+  const prCoAuthors = await fetchPRCoAuthors(
+    withoutCoAuthors.map(commit => String(commit.pr)),
+  );
+  for (const commit of withoutCoAuthors) {
+    commit.coAuthors = prCoAuthors[String(commit.pr)] ?? commit.coAuthors;
+  }
+
   return result;
 }