From 9c2ff3982b8c0566eb520909c549b27a505df999 Mon Sep 17 00:00:00 2001 From: Joris Blaak Date: Mon, 23 Mar 2026 11:37:37 +0100 Subject: [PATCH] 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) --- src/Github/Api/getPullRequestService.ts | 2 +- .../githubPullRequestInfoProvider.test.ts | 41 +++++++++++ src/Github/githubPullRequestInfoProvider.ts | 72 ++++++++++--------- 3 files changed, 80 insertions(+), 35 deletions(-) diff --git a/src/Github/Api/getPullRequestService.ts b/src/Github/Api/getPullRequestService.ts index 667986d..b74b47f 100644 --- a/src/Github/Api/getPullRequestService.ts +++ b/src/Github/Api/getPullRequestService.ts @@ -7,7 +7,7 @@ export interface GetPullRequestService { export interface ApiGetPullRequest { draft: boolean; - rebaseable: boolean; + rebaseable: boolean | null; mergeableState: MergeableState; labels: string[]; } diff --git a/src/Github/__tests__/githubPullRequestInfoProvider.test.ts b/src/Github/__tests__/githubPullRequestInfoProvider.test.ts index 4b1d1ec..79ab6d9 100644 --- a/src/Github/__tests__/githubPullRequestInfoProvider.test.ts +++ b/src/Github/__tests__/githubPullRequestInfoProvider.test.ts @@ -91,4 +91,45 @@ describe('The pull request info is retried', () => { /* Then */ expect(result.mergeableState).toBe('unknown'); }); + + it('when rebaseable is null', async () => { + /* Given */ + getPullRequestService.results.push({ + draft: false, + rebaseable: null, + mergeableState: 'behind', + labels: [], + }); + + getPullRequestService.results.push({ + draft: false, + rebaseable: true, + mergeableState: 'behind', + labels: [], + }); + + /* When */ + const result = await provider.pullRequestInfoFor('owner', 'repo', 3); + + /* Then */ + expect(result.rebaseable).toBe(true); + }); + + it('when rebaseable is null after max retries, defaults to false', async () => { + /* Given */ + for (let i = 0; i < 10; i++) { + getPullRequestService.results.push({ + draft: false, + rebaseable: null, + mergeableState: 'behind', + labels: [], + }); + } + + /* When */ + const result = await provider.pullRequestInfoFor('owner', 'repo', 3); + + /* Then */ + expect(result.rebaseable).toBe(false); + }); }); diff --git a/src/Github/githubPullRequestInfoProvider.ts b/src/Github/githubPullRequestInfoProvider.ts index 7bb7a16..d36c05c 100644 --- a/src/Github/githubPullRequestInfoProvider.ts +++ b/src/Github/githubPullRequestInfoProvider.ts @@ -11,43 +11,47 @@ export class GithubPullRequestInfoProvider { repoName: string, pullRequestNumber: number, ): Promise { - return promiseRetry( - async (attemptNumber): Promise => { - try { - const {draft, rebaseable, mergeableState, labels} = await this.getPullRequestService.getPullRequest( - ownerName, - repoName, - pullRequestNumber, - ); + return promiseRetry(async (attemptNumber): Promise => { + try { + const {draft, rebaseable, mergeableState, labels} = await this.getPullRequestService.getPullRequest( + ownerName, + repoName, + pullRequestNumber, + ); - if (attemptNumber < 10 && !draft) { - if (mergeableState === 'unknown' || !mergeableStates.includes(mergeableState)) { - debug(`mergeableState for pull request #${pullRequestNumber} is 'unknown', retrying.`); - throw Error("mergeableState is 'unknown'"); - } + if (attemptNumber < 10 && !draft) { + if (mergeableState === 'unknown' || !mergeableStates.includes(mergeableState)) { + debug(`mergeableState for pull request #${pullRequestNumber} is 'unknown', retrying.`); + throw Error("mergeableState is 'unknown'"); } + if (rebaseable === null) { + debug( + `rebaseable for pull request #${pullRequestNumber} is null (not yet computed), retrying.`, + ); + throw Error('rebaseable is null'); + } + } - debug(`rebaseable value for pull request #${pullRequestNumber}: ${String(rebaseable)}`); - debug(`mergeableState for pull request #${pullRequestNumber}: ${mergeableState}`); + debug(`rebaseable value for pull request #${pullRequestNumber}: ${String(rebaseable)}`); + debug(`mergeableState for pull request #${pullRequestNumber}: ${mergeableState}`); - return { - ownerName: ownerName, - repoName: repoName, - number: pullRequestNumber, - draft: draft, - rebaseable: rebaseable, - mergeableState: mergeableState, - labels: labels, - }; - } catch (error) { - debug( - `Fetching mergeableState for pull request #${pullRequestNumber} failed: "${String( - error, - )}", retrying.`, - ); - throw error; - } - }, - ); + return { + ownerName: ownerName, + repoName: repoName, + number: pullRequestNumber, + draft: draft, + rebaseable: rebaseable ?? false, + mergeableState: mergeableState, + labels: labels, + }; + } catch (error) { + debug( + `Fetching mergeableState for pull request #${pullRequestNumber} failed: "${String( + error, + )}", retrying.`, + ); + throw error; + } + }); } }