diff --git a/src/github.ts b/src/github.ts index 753f0cd75..5b120450f 100644 --- a/src/github.ts +++ b/src/github.ts @@ -1567,6 +1567,21 @@ export class GitHub { .toString() .slice(0, MAX_ISSUE_BODY_SIZE); + // re-fetch before trying to recreate the PR, in case an existing one + // was merged while this was running. this avoids modifying a PR that's + // already closed. + const currentPr = await this.octokit.pulls.get({ + owner: this.repository.owner, + repo: this.repository.repo, + pull_number: number, + }); + if (currentPr.data.state !== 'open') { + this.logger.warn( + `Pull request #${number} is no longer open (state: ${currentPr.data.state}) - skipping update` + ); + return this.getPullRequest(number); + } + await this.createPullRequest( { headBranchName: releasePullRequest.headRefName, diff --git a/test/github.ts b/test/github.ts index 2a214462e..d5cdb3eb0 100644 --- a/test/github.ts +++ b/test/github.ts @@ -1128,18 +1128,25 @@ describe('GitHub', () => { files: [], }); - req = req.patch('/repos/fake/fake/pulls/123').reply(200, { - number: 123, - title: 'updated-title', - body: 'updated body', - labels: [], - head: { - ref: 'release-please--branches--main--changes--next', - }, - base: { - ref: 'main', - }, - }); + req = req + .get('/repos/fake/fake/pulls/123') + .reply(200, { + number: 123, + state: 'open', + }) + .patch('/repos/fake/fake/pulls/123') + .reply(200, { + number: 123, + title: 'updated-title', + body: 'updated body', + labels: [], + head: { + ref: 'release-please--branches--main--changes--next', + }, + base: { + ref: 'main', + }, + }); const pullRequest: ReleasePullRequest = { title: PullRequestTitle.ofTargetBranch('main', 'next'), @@ -1164,18 +1171,25 @@ describe('GitHub', () => { .stub(github, 'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any .resolves('the-pull-request-branch-sha'); - req = req.patch('/repos/fake/fake/pulls/123').reply(200, { - number: 123, - title: 'updated-title', - body: 'updated body', - labels: [], - head: { - ref: 'release-please--branches--main', - }, - base: { - ref: 'main', - }, - }); + req = req + .get('/repos/fake/fake/pulls/123') + .reply(200, { + number: 123, + state: 'open', + }) + .patch('/repos/fake/fake/pulls/123') + .reply(200, { + number: 123, + title: 'updated-title', + body: 'updated body', + labels: [], + head: { + ref: 'release-please--branches--main', + }, + base: { + ref: 'main', + }, + }); const getPullRequestStub = sandbox .stub(github, 'getPullRequest') .withArgs(123) @@ -1209,6 +1223,58 @@ describe('GitHub', () => { sinon.assert.calledOnce(getPullRequestStub); req.done(); }); + + it('skips update if the PR was merged between query and update', async () => { + const upsertReleaseBranch = sandbox + .stub(github, 'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any + .resolves('the-pull-request-branch-sha'); + + const getPullRequestStub = sandbox + .stub(github, 'getPullRequest') + .withArgs(123) + .resolves({ + title: 'release: 1.9.1', + headBranchName: 'release-please--branches--main--changes--next', + baseBranchName: 'main', + number: 123, + body: 'release body', + labels: ['autorelease: tagged'], + files: [], + }); + + // PR was merged between the initial open-PR query and this update + req = req.get('/repos/fake/fake/pulls/123').reply(200, { + number: 123, + state: 'closed', + merged: true, + }); + + const pullRequest: ReleasePullRequest = { + title: PullRequestTitle.ofTargetBranch('main', 'next'), + body: new PullRequestBody(mockReleaseData(1000), { + useComponents: true, + }), + labels: ['autorelease: pending'], + headRefName: 'release-please--branches--main--changes--next', + draft: false, + updates: [], + conventionalCommits: [], + }; + + const result = await github.updatePullRequest( + 123, + pullRequest, + 'main', + 'next' + ); + + // Should return the current PR state without modifying it + sinon.assert.notCalled(upsertReleaseBranch); + sinon.assert.calledOnce(getPullRequestStub); + expect(result.number).to.equal(123); + expect(result.labels).to.include('autorelease: tagged'); + req.done(); + }); }); describe('createPullRequest', () => {