Skip to content

Commit 867df90

Browse files
committed
fix(github): retry the transient 405 'Merge already in progress' race (#5003)
classifyMergeFailure treated every 405 except the known "Base branch was modified" TOCTOU race as terminal, holding the PR for a human. "Merge already in progress" (GITTENSORY-1K, 2 Sentry events) is the same class of benign race -- another merge request for the same PR is already being processed by GitHub -- not a policy rejection, so it resolves the same way: retry against the settled state instead of holding.
1 parent afc7993 commit 867df90

2 files changed

Lines changed: 16 additions & 0 deletions

File tree

src/services/merge-failure.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,15 @@ function isBaseBranchMovedMessage(message: string): boolean {
3333
return /base branch was modified/i.test(message);
3434
}
3535

36+
/** True for the transient "Merge already in progress" 405 (GITTENSORY-1K) — another merge request for the
37+
* SAME PR (a manual click, a concurrent duplicate job) is already being processed by GitHub. Not a policy
38+
* rejection: the in-flight merge either lands (making this retry a no-op once the PR is no longer open) or
39+
* fails (making a retry the right move), so it resolves the same way isBaseBranchMovedMessage's TOCTOU race
40+
* does — re-attempt rather than hold. */
41+
function isMergeAlreadyInProgressMessage(message: string): boolean {
42+
return /merge already in progress/i.test(message);
43+
}
44+
3645
function isConvergenceForbiddenMessage(message: string): boolean {
3746
return /resource not accessible by integration|secondary rate limit|api rate limit|abuse detection/i.test(message);
3847
}
@@ -55,6 +64,7 @@ export function classifyMergeFailure(error: unknown): { terminal: boolean; reaso
5564
// A 405 "Base branch was modified" is a benign TOCTOU race, not a policy rejection — retry against the new base
5665
// (the executor caps retries at MERGE_RETRY_CAP before escalating to the same terminal hold).
5766
if (status === 405 && isBaseBranchMovedMessage(message)) return { terminal: false, reason: `base branch moved during merge — retrying: ${message}` };
67+
if (status === 405 && isMergeAlreadyInProgressMessage(message)) return { terminal: false, reason: `a merge for this PR was already in progress — retrying: ${message}` };
5868
if (status === 405) return { terminal: true, reason: `merge not allowed (405 — repo merge policy forbids an automated merge): ${message}` };
5969
if (status === 409) return { terminal: true, reason: `merge conflict / required check absent (409): ${message}` };
6070
if (isMergeConflictMessage(message)) return { terminal: true, reason: `branch conflicts with base — contributor must rebase: ${message}` };

test/unit/merge-failure.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ describe("classifyMergeFailure", () => {
1313
expect(result.reason).toMatch(/base branch moved/i);
1414
});
1515

16+
it("REGRESSION (#5003, GITTENSORY-1K): retries the transient 405 'Merge already in progress' race instead of holding it", () => {
17+
const result = classifyMergeFailure(httpError(405, "Merge already in progress"));
18+
expect(result.terminal).toBe(false);
19+
expect(result.reason).toMatch(/already in progress/i);
20+
});
21+
1622
it("still treats a policy 405 (required reviews/checks) as terminal", () => {
1723
const result = classifyMergeFailure(httpError(405, "At least 1 approving review is required by reviewers with write access."));
1824
expect(result.terminal).toBe(true);

0 commit comments

Comments
 (0)