root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): bounded retry loop for push-after-merge non-fast-forward rejections (#1944)

Browse Source 
## Summary
- In direct-merge mode with `pushAfterMerge: true`,
`pushToRemoteAfterMerge` previously did one preemptive `pull --rebase`
before the first push, then allowed exactly **one** additional retry
when the push failed as non-fast-forward.
- On a busy repo, origin can move again during that single retry's
pull/push window, so the retry itself can also lose the race — leaving
the merge unpushed with no further attempt.
- This generalizes the single retry into a bounded loop
(`PUSH_NON_FF_MAX_RETRIES = 3`, backoff `2s/5s/10s`), re-running
`pullWithRebaseAndResolveConflicts` before each attempt, and breaking
out early if a retry's failure is no longer classified as
non-fast-forward (so unrelated errors surface immediately instead of
being retried needlessly).
- Abort-signal checks (`throwIfAborted`) and merge-abort rethrow
(`rethrowIfMergeAborted`) are preserved at every step of the loop.

## Test plan
- [x] Existing test `"retries push once after non-fast-forward
rejection"` still passes unmodified (loop returns on first successful
retry, same attempt counts as before).
- [x] New test `"retries push multiple times across repeated
non-fast-forward rejections"` — 2 consecutive non-ff failures then
success on the 3rd push attempt, proving the loop goes beyond the old
single-retry ceiling.
- [x] New test `"gives up after exhausting non-fast-forward retries"` —
all attempts fail as non-ff, proving retries are bounded (`pushed:
false` after 1 initial + 3 retries) rather than looping forever.
- [x] `packages/engine` full test suite: 37/37 passing (`npx vitest run
src/__tests__/merger-prompt-and-utils.test.ts`).
- [x] `npx tsc --noEmit -p .` clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved reliability when sending merged changes to a remote by
retrying after non-fast-forward push failures.
* Added configurable backoff and retry limits to recover via pull +
rebase and provide a clear failure when retries are exhausted.
* **Tests**
* Added coverage for repeated non-fast-forward retries, including the
case where the retry limit is reached and the operation ultimately
fails.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
gsxdsm
2026-07-07 22:03:59 -07:00
committed by GitHub
parent 0782184f07 860e42d1bf
commit 32095a74cc
2 changed files with 124 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
packages/engine/src/__tests__/merger-prompt-and-utils.test.ts
View File

@@ -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;

66
packages/engine/src/merger.ts
View File

@@ -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<void> {
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 };
}
}