From 77cefbcd1524ac414e69e35658c141e4ead210ec Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 7 Nov 2026 01:07:47 +0000 Subject: [PATCH 1/2] fix(changelog): Credit PR commit co-authors when the merge commit has none Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com> --- docs/src/content/docs/changelogs.md | 3 +- .../__tests__/changelog-generate.test.ts | 65 ++++++++++++- src/utils/changelog.ts | 94 +++++++++++++------ 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/docs/src/content/docs/changelogs.md b/docs/src/content/docs/changelogs.md index 226e2532..0702f729 100644 --- a/docs/src/content/docs/changelogs.md +++ b/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`: diff --git a/src/utils/__tests__/changelog-generate.test.ts b/src/utils/__tests__/changelog-generate.test.ts index 3c211f52..c79cb569 100644 --- a/src/utils/__tests__/changelog-generate.test.ts +++ b/src/utils/__tests__/changelog-generate.test.ts @@ -912,6 +912,69 @@ describe('generateChangesetFromGit', () => { ); }); + it('credits PR commit co-authors when the merged commit has none', async () => { + setup([junior([])], 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 +2507,7 @@ describe('generateChangelogWithHighlight', () => { }); mockGraphqlClient.mockResolvedValueOnce({ repository: { - pullRequest: { + P7: { commits: { nodes: [ { commit: { authors: commitAuthors(['bob']) } }, diff --git a/src/utils/changelog.ts b/src/utils/changelog.ts index ab699296..ec6fff85 100644 --- a/src/utils/changelog.ts +++ b/src/utils/changelog.ts @@ -107,6 +107,14 @@ interface CommitAuthors { nodes: Array<{ user: { login: string } | null }>; } +/** PRs keyed by `P` 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,20 +124,24 @@ 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 { +async function fetchPRCoAuthors( + prNumbers: string[], +): Promise> { 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 = {}; + 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 { @@ -137,26 +149,36 @@ async function fetchPRCoAuthors(prNumber: number): Promise { } } } + }`, + ) + .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,16 @@ 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. + const prsWithoutCoAuthors = [...seenPRs] + .filter(([, commit]) => !commit.coAuthors?.length) + .map(([pr]) => pr); + const prCoAuthors = await fetchPRCoAuthors(prsWithoutCoAuthors); + for (const [pr, coAuthors] of Object.entries(prCoAuthors)) { + seenPRs.get(pr)!.coAuthors = coAuthors; + } + return result; } From 3671be458c313d5cd77784126373f82dc2377818 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 7 Nov 2026 01:13:12 +0000 Subject: [PATCH 2/2] fix(changelog): Match PR co-authors when GraphQL returns numeric PR numbers --- src/utils/__tests__/changelog-generate.test.ts | 5 ++++- src/utils/changelog.ts | 16 ++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/utils/__tests__/changelog-generate.test.ts b/src/utils/__tests__/changelog-generate.test.ts index c79cb569..d7e8202a 100644 --- a/src/utils/__tests__/changelog-generate.test.ts +++ b/src/utils/__tests__/changelog-generate.test.ts @@ -913,7 +913,10 @@ describe('generateChangesetFromGit', () => { }); it('credits PR commit co-authors when the merged commit has none', async () => { - setup([junior([])], null); + 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: { diff --git a/src/utils/changelog.ts b/src/utils/changelog.ts index ec6fff85..44bdbee2 100644 --- a/src/utils/changelog.ts +++ b/src/utils/changelog.ts @@ -1979,12 +1979,16 @@ async function fetchRawCommitInfo( // 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. - const prsWithoutCoAuthors = [...seenPRs] - .filter(([, commit]) => !commit.coAuthors?.length) - .map(([pr]) => pr); - const prCoAuthors = await fetchPRCoAuthors(prsWithoutCoAuthors); - for (const [pr, coAuthors] of Object.entries(prCoAuthors)) { - seenPRs.get(pr)!.coAuthors = coAuthors; + // 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;