Skip to content

Commit 9c2ff39

Browse files
JBlaakclaude
andcommitted
Fix false "not rebaseable" by retrying when GitHub returns null
GitHub's API computes rebaseable asynchronously and returns null while pending. The code already retried for unknown mergeableState but treated null rebaseable as false, causing eligible PRs to be skipped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a96444d commit 9c2ff39

3 files changed

Lines changed: 80 additions & 35 deletions

File tree

src/Github/Api/getPullRequestService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export interface GetPullRequestService {
77

88
export interface ApiGetPullRequest {
99
draft: boolean;
10-
rebaseable: boolean;
10+
rebaseable: boolean | null;
1111
mergeableState: MergeableState;
1212
labels: string[];
1313
}

src/Github/__tests__/githubPullRequestInfoProvider.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,45 @@ describe('The pull request info is retried', () => {
9191
/* Then */
9292
expect(result.mergeableState).toBe('unknown');
9393
});
94+
95+
it('when rebaseable is null', async () => {
96+
/* Given */
97+
getPullRequestService.results.push({
98+
draft: false,
99+
rebaseable: null,
100+
mergeableState: 'behind',
101+
labels: [],
102+
});
103+
104+
getPullRequestService.results.push({
105+
draft: false,
106+
rebaseable: true,
107+
mergeableState: 'behind',
108+
labels: [],
109+
});
110+
111+
/* When */
112+
const result = await provider.pullRequestInfoFor('owner', 'repo', 3);
113+
114+
/* Then */
115+
expect(result.rebaseable).toBe(true);
116+
});
117+
118+
it('when rebaseable is null after max retries, defaults to false', async () => {
119+
/* Given */
120+
for (let i = 0; i < 10; i++) {
121+
getPullRequestService.results.push({
122+
draft: false,
123+
rebaseable: null,
124+
mergeableState: 'behind',
125+
labels: [],
126+
});
127+
}
128+
129+
/* When */
130+
const result = await provider.pullRequestInfoFor('owner', 'repo', 3);
131+
132+
/* Then */
133+
expect(result.rebaseable).toBe(false);
134+
});
94135
});

src/Github/githubPullRequestInfoProvider.ts

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,43 +11,47 @@ export class GithubPullRequestInfoProvider {
1111
repoName: string,
1212
pullRequestNumber: number,
1313
): Promise<PullRequestInfo> {
14-
return promiseRetry<PullRequestInfo>(
15-
async (attemptNumber): Promise<PullRequestInfo> => {
16-
try {
17-
const {draft, rebaseable, mergeableState, labels} = await this.getPullRequestService.getPullRequest(
18-
ownerName,
19-
repoName,
20-
pullRequestNumber,
21-
);
14+
return promiseRetry<PullRequestInfo>(async (attemptNumber): Promise<PullRequestInfo> => {
15+
try {
16+
const {draft, rebaseable, mergeableState, labels} = await this.getPullRequestService.getPullRequest(
17+
ownerName,
18+
repoName,
19+
pullRequestNumber,
20+
);
2221

23-
if (attemptNumber < 10 && !draft) {
24-
if (mergeableState === 'unknown' || !mergeableStates.includes(mergeableState)) {
25-
debug(`mergeableState for pull request #${pullRequestNumber} is 'unknown', retrying.`);
26-
throw Error("mergeableState is 'unknown'");
27-
}
22+
if (attemptNumber < 10 && !draft) {
23+
if (mergeableState === 'unknown' || !mergeableStates.includes(mergeableState)) {
24+
debug(`mergeableState for pull request #${pullRequestNumber} is 'unknown', retrying.`);
25+
throw Error("mergeableState is 'unknown'");
2826
}
27+
if (rebaseable === null) {
28+
debug(
29+
`rebaseable for pull request #${pullRequestNumber} is null (not yet computed), retrying.`,
30+
);
31+
throw Error('rebaseable is null');
32+
}
33+
}
2934

30-
debug(`rebaseable value for pull request #${pullRequestNumber}: ${String(rebaseable)}`);
31-
debug(`mergeableState for pull request #${pullRequestNumber}: ${mergeableState}`);
35+
debug(`rebaseable value for pull request #${pullRequestNumber}: ${String(rebaseable)}`);
36+
debug(`mergeableState for pull request #${pullRequestNumber}: ${mergeableState}`);
3237

33-
return {
34-
ownerName: ownerName,
35-
repoName: repoName,
36-
number: pullRequestNumber,
37-
draft: draft,
38-
rebaseable: rebaseable,
39-
mergeableState: mergeableState,
40-
labels: labels,
41-
};
42-
} catch (error) {
43-
debug(
44-
`Fetching mergeableState for pull request #${pullRequestNumber} failed: "${String(
45-
error,
46-
)}", retrying.`,
47-
);
48-
throw error;
49-
}
50-
},
51-
);
38+
return {
39+
ownerName: ownerName,
40+
repoName: repoName,
41+
number: pullRequestNumber,
42+
draft: draft,
43+
rebaseable: rebaseable ?? false,
44+
mergeableState: mergeableState,
45+
labels: labels,
46+
};
47+
} catch (error) {
48+
debug(
49+
`Fetching mergeableState for pull request #${pullRequestNumber} failed: "${String(
50+
error,
51+
)}", retrying.`,
52+
);
53+
throw error;
54+
}
55+
});
5256
}
5357
}

0 commit comments

Comments
 (0)