diff --git a/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts b/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts index 5d79e05c38..a1c0e6864f 100644 --- a/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts +++ b/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts @@ -731,6 +731,87 @@ describe("push-after-merge", () => { expect(pushAttempts).toBe(2); }); + it("retries push multiple times across repeated non-fast-forward rejections", async () => { + let pushAttempts = 0; + let pullAttempts = 0; + + mockedExecSync.mockImplementation((cmd: any) => { + const cmdStr = String(cmd); + if (cmdStr.startsWith('git pull --rebase "origin" "main"')) { + pullAttempts += 1; + return Buffer.from(""); + } + if (cmdStr.startsWith('git push "origin" "main"')) { + pushAttempts += 1; + if (pushAttempts < 3) { + const err = new Error("non-fast-forward") as Error & { stderr?: string }; + err.stderr = "[rejected] main -> main (non-fast-forward)"; + throw err; + } + return Buffer.from(""); + } + if (cmdStr.includes("git symbolic-ref --short HEAD")) return "main" as any; + if (cmdStr.includes("git rev-parse --verify REBASE_HEAD")) { + const err = new Error("fatal: Needed a single revision") as Error & { status?: number }; + err.status = 128; + throw err; + } + return Buffer.from(""); + }); + + const store = createMockStore(); + const result = await pushToRemoteAfterMerge(store, "/tmp/root", "FN-050", { + ...DEFAULT_SETTINGS, + mergeIntegrationWorktree: "cwd-main" as const, + pushAfterMerge: true, + pushRemote: "origin", + }); + + expect(result.pushed).toBe(true); + expect(pullAttempts).toBe(3); + expect(pushAttempts).toBe(3); + }); + + it("gives up after exhausting non-fast-forward retries", async () => { + let pushAttempts = 0; + let pullAttempts = 0; + + mockedExecSync.mockImplementation((cmd: any) => { + const cmdStr = String(cmd); + if (cmdStr.startsWith('git pull --rebase "origin" "main"')) { + pullAttempts += 1; + return Buffer.from(""); + } + if (cmdStr.startsWith('git push "origin" "main"')) { + pushAttempts += 1; + const err = new Error("non-fast-forward") as Error & { stderr?: string }; + err.stderr = `[rejected] main -> main (non-fast-forward) attempt ${pushAttempts}`; + throw err; + } + if (cmdStr.includes("git symbolic-ref --short HEAD")) return "main" as any; + if (cmdStr.includes("git rev-parse --verify REBASE_HEAD")) { + const err = new Error("fatal: Needed a single revision") as Error & { status?: number }; + err.status = 128; + throw err; + } + return Buffer.from(""); + }); + + const store = createMockStore(); + const result = await pushToRemoteAfterMerge(store, "/tmp/root", "FN-050", { + ...DEFAULT_SETTINGS, + mergeIntegrationWorktree: "cwd-main" as const, + pushAfterMerge: true, + pushRemote: "origin", + }); + + expect(result.pushed).toBe(false); + expect(result.error).toContain("non-fast-forward"); + // 1 initial push attempt + PUSH_NON_FF_MAX_RETRIES (3) retries = 4 total + expect(pushAttempts).toBe(4); + expect(pullAttempts).toBe(4); + }); + it("aborts rebase when conflicts remain unresolved", async () => { let rebaseInProgress = false; diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index c698cd8ed1..099dc53605 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -342,6 +342,12 @@ const DEPENDENCY_SYNC_TRIGGER_PATTERNS = [ const PULL_REBASE_TIMEOUT_MS = 120_000; const PUSH_TIMEOUT_MS = 60_000; +const PUSH_NON_FF_MAX_RETRIES = 3; +const PUSH_NON_FF_RETRY_BACKOFF_MS = [2_000, 5_000, 10_000]; + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} export async function emitMergeAttemptAuditEvent(params: { audit: RunAuditor; @@ -7336,33 +7342,47 @@ export async function pushToRemoteAfterMerge( mergerLog.log(`${taskId}: pushed merged result to ${remote}/${branch}`); return { pushed: true }; } catch (firstPushError: unknown) { - const firstMessage = getCommandErrorMessage(firstPushError); - mergerLog.warn(`${taskId}: initial push failed: ${firstMessage}`); + let lastMessage = getCommandErrorMessage(firstPushError); + mergerLog.warn(`${taskId}: initial push failed: ${lastMessage}`); - if (!isNonFastForwardPushError(firstMessage)) { - return { pushed: false, error: firstMessage }; + if (!isNonFastForwardPushError(lastMessage)) { + return { pushed: false, error: lastMessage }; } - mergerLog.log(`${taskId}: push rejected as non-fast-forward; retrying pull --rebase and push once`); - - try { - throwIfAborted(options?.signal, taskId); - await pullWithRebaseAndResolveConflicts(store, rootDir, taskId, settings, remote, branch, options); - throwIfAborted(options?.signal, taskId); - await execAsync(pushCommand, { - cwd: rootDir, - timeout: PUSH_TIMEOUT_MS, - maxBuffer: VERIFICATION_COMMAND_MAX_BUFFER, - encoding: "utf-8", - }); - mergerLog.log(`${taskId}: push succeeded after non-fast-forward retry`); - return { pushed: true }; - } catch (retryError: unknown) { - rethrowIfMergeAborted(retryError); - const retryMessage = getCommandErrorMessage(retryError); - mergerLog.error(`${taskId}: push retry failed: ${retryMessage}`); - return { pushed: false, error: retryMessage }; + // Non-fast-forward push failures mean origin moved between our pre-push + // pull and the push itself. A single retry can still lose the race if + // origin moves again in that window (busy repos, concurrent mergers), so + // retry a bounded number of times with backoff, re-fetching+rebasing + // before each attempt. + const maxRetries = PUSH_NON_FF_MAX_RETRIES; + for (let attempt = 1; attempt <= maxRetries; attempt++) { + mergerLog.log( + `${taskId}: push rejected as non-fast-forward; retrying pull --rebase and push (attempt ${attempt}/${maxRetries})`, + ); + try { + throwIfAborted(options?.signal, taskId); + await pullWithRebaseAndResolveConflicts(store, rootDir, taskId, settings, remote, branch, options); + throwIfAborted(options?.signal, taskId); + await execAsync(pushCommand, { + cwd: rootDir, + timeout: PUSH_TIMEOUT_MS, + maxBuffer: VERIFICATION_COMMAND_MAX_BUFFER, + encoding: "utf-8", + }); + mergerLog.log(`${taskId}: push succeeded after non-fast-forward retry (attempt ${attempt}/${maxRetries})`); + return { pushed: true }; + } catch (retryError: unknown) { + rethrowIfMergeAborted(retryError); + lastMessage = getCommandErrorMessage(retryError); + mergerLog.error(`${taskId}: push retry ${attempt}/${maxRetries} failed: ${lastMessage}`); + if (attempt === maxRetries || !isNonFastForwardPushError(lastMessage)) { + break; + } + throwIfAborted(options?.signal, taskId); + await delay(PUSH_NON_FF_RETRY_BACKOFF_MS[attempt - 1] ?? PUSH_NON_FF_RETRY_BACKOFF_MS.at(-1)!); + } } + return { pushed: false, error: lastMessage }; } }