Skip to content

Commit a116e1e

Browse files
authored
fix: check for closed pr before recreating/relabeling (#220)
1 parent 8427ac1 commit a116e1e

2 files changed

Lines changed: 105 additions & 24 deletions

File tree

src/github.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,21 @@ export class GitHub {
15671567
.toString()
15681568
.slice(0, MAX_ISSUE_BODY_SIZE);
15691569

1570+
// re-fetch before trying to recreate the PR, in case an existing one
1571+
// was merged while this was running. this avoids modifying a PR that's
1572+
// already closed.
1573+
const currentPr = await this.octokit.pulls.get({
1574+
owner: this.repository.owner,
1575+
repo: this.repository.repo,
1576+
pull_number: number,
1577+
});
1578+
if (currentPr.data.state !== 'open') {
1579+
this.logger.warn(
1580+
`Pull request #${number} is no longer open (state: ${currentPr.data.state}) - skipping update`
1581+
);
1582+
return this.getPullRequest(number);
1583+
}
1584+
15701585
await this.createPullRequest(
15711586
{
15721587
headBranchName: releasePullRequest.headRefName,

test/github.ts

Lines changed: 90 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,18 +1128,25 @@ describe('GitHub', () => {
11281128
files: [],
11291129
});
11301130

1131-
req = req.patch('/repos/fake/fake/pulls/123').reply(200, {
1132-
number: 123,
1133-
title: 'updated-title',
1134-
body: 'updated body',
1135-
labels: [],
1136-
head: {
1137-
ref: 'release-please--branches--main--changes--next',
1138-
},
1139-
base: {
1140-
ref: 'main',
1141-
},
1142-
});
1131+
req = req
1132+
.get('/repos/fake/fake/pulls/123')
1133+
.reply(200, {
1134+
number: 123,
1135+
state: 'open',
1136+
})
1137+
.patch('/repos/fake/fake/pulls/123')
1138+
.reply(200, {
1139+
number: 123,
1140+
title: 'updated-title',
1141+
body: 'updated body',
1142+
labels: [],
1143+
head: {
1144+
ref: 'release-please--branches--main--changes--next',
1145+
},
1146+
base: {
1147+
ref: 'main',
1148+
},
1149+
});
11431150

11441151
const pullRequest: ReleasePullRequest = {
11451152
title: PullRequestTitle.ofTargetBranch('main', 'next'),
@@ -1164,18 +1171,25 @@ describe('GitHub', () => {
11641171
.stub(github, <any>'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
11651172
.resolves('the-pull-request-branch-sha');
11661173

1167-
req = req.patch('/repos/fake/fake/pulls/123').reply(200, {
1168-
number: 123,
1169-
title: 'updated-title',
1170-
body: 'updated body',
1171-
labels: [],
1172-
head: {
1173-
ref: 'release-please--branches--main',
1174-
},
1175-
base: {
1176-
ref: 'main',
1177-
},
1178-
});
1174+
req = req
1175+
.get('/repos/fake/fake/pulls/123')
1176+
.reply(200, {
1177+
number: 123,
1178+
state: 'open',
1179+
})
1180+
.patch('/repos/fake/fake/pulls/123')
1181+
.reply(200, {
1182+
number: 123,
1183+
title: 'updated-title',
1184+
body: 'updated body',
1185+
labels: [],
1186+
head: {
1187+
ref: 'release-please--branches--main',
1188+
},
1189+
base: {
1190+
ref: 'main',
1191+
},
1192+
});
11791193
const getPullRequestStub = sandbox
11801194
.stub(github, 'getPullRequest')
11811195
.withArgs(123)
@@ -1209,6 +1223,58 @@ describe('GitHub', () => {
12091223
sinon.assert.calledOnce(getPullRequestStub);
12101224
req.done();
12111225
});
1226+
1227+
it('skips update if the PR was merged between query and update', async () => {
1228+
const upsertReleaseBranch = sandbox
1229+
.stub(github, <any>'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
1230+
.resolves('the-pull-request-branch-sha');
1231+
1232+
const getPullRequestStub = sandbox
1233+
.stub(github, 'getPullRequest')
1234+
.withArgs(123)
1235+
.resolves({
1236+
title: 'release: 1.9.1',
1237+
headBranchName: 'release-please--branches--main--changes--next',
1238+
baseBranchName: 'main',
1239+
number: 123,
1240+
body: 'release body',
1241+
labels: ['autorelease: tagged'],
1242+
files: [],
1243+
});
1244+
1245+
// PR was merged between the initial open-PR query and this update
1246+
req = req.get('/repos/fake/fake/pulls/123').reply(200, {
1247+
number: 123,
1248+
state: 'closed',
1249+
merged: true,
1250+
});
1251+
1252+
const pullRequest: ReleasePullRequest = {
1253+
title: PullRequestTitle.ofTargetBranch('main', 'next'),
1254+
body: new PullRequestBody(mockReleaseData(1000), {
1255+
useComponents: true,
1256+
}),
1257+
labels: ['autorelease: pending'],
1258+
headRefName: 'release-please--branches--main--changes--next',
1259+
draft: false,
1260+
updates: [],
1261+
conventionalCommits: [],
1262+
};
1263+
1264+
const result = await github.updatePullRequest(
1265+
123,
1266+
pullRequest,
1267+
'main',
1268+
'next'
1269+
);
1270+
1271+
// Should return the current PR state without modifying it
1272+
sinon.assert.notCalled(upsertReleaseBranch);
1273+
sinon.assert.calledOnce(getPullRequestStub);
1274+
expect(result.number).to.equal(123);
1275+
expect(result.labels).to.include('autorelease: tagged');
1276+
req.done();
1277+
});
12121278
});
12131279

12141280
describe('createPullRequest', () => {

0 commit comments

Comments
 (0)