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

Merge pull request #68 from bluk1020/fix/already-merged-pr-reconcile

Browse Source 
fix: reconcile PR merge race
This commit is contained in:
gsxdsm
2026-05-10 23:48:23 -07:00
committed by GitHub
parent 0e5bdf338a 457ea59a0d
commit f14709f449
3 changed files with 230 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

193
packages/cli/src/commands/__tests__/task-lifecycle.test.ts
View File

@@ -502,6 +502,199 @@ describe("processPullRequestMergeTask", () => {
expect(store.moveTask).toHaveBeenCalledWith("FN-9004", "done");
});
it("reconciles to done when PR merges after readiness check but before merge command completes", async () => {
const task: MockTask = {
id: "FN-9104",
title: "test",
description: "desc",
column: "in-review",
worktree: "/tmp/worktree-fn-9104",
prInfo: {
number: 124,
url: "https://github.com/x/y/pull/124",
status: "open",
headBranch: "fusion/fn-9104",
baseBranch: "main",
},
};
const store = makeStore(task);
execMock.mockImplementation(() => "");
const openPr = {
number: 124,
url: "https://github.com/x/y/pull/124",
status: "open" as const,
headBranch: "fusion/fn-9104",
baseBranch: "main",
};
const mergedPr = {
...openPr,
status: "merged" as const,
};
const github = {
findPrForBranch: vi.fn(),
createPr: vi.fn(),
getPrMergeStatus: vi
.fn()
.mockResolvedValueOnce({
prInfo: openPr,
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
})
.mockResolvedValueOnce({
prInfo: mergedPr,
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
}),
mergePr: vi.fn(async () => {
throw new Error("Pull request is not mergeable: the merge commit cannot be cleanly created");
}),
};
const result = await processPullRequestMergeTask(
store as never,
"/repo",
task.id,
github as never,
() => undefined,
);
expect(result).toBe("merged");
expect(github.mergePr).toHaveBeenCalledWith({ number: 124, method: "squash" });
expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2);
expect(store.updatePrInfo).toHaveBeenLastCalledWith("FN-9104", expect.objectContaining({ status: "merged" }));
expect(store.updateTask).toHaveBeenCalledWith("FN-9104", { status: null, mergeRetries: 0 });
expect(store.moveTask).toHaveBeenCalledWith("FN-9104", "done");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-9104",
"Pull request already merged after merge command failed; reconciled task state from GitHub",
"PR #124: https://github.com/x/y/pull/124",
);
});
it("rethrows the original merge error when refresh does not confirm merged", async () => {
const task: MockTask = {
id: "FN-9105",
title: "test",
description: "desc",
column: "in-review",
prInfo: {
number: 125,
url: "https://github.com/x/y/pull/125",
status: "open",
headBranch: "fusion/fn-9105",
baseBranch: "main",
},
};
const store = makeStore(task);
const mergeError = new Error("Pull request is not mergeable");
const openPr = {
number: 125,
url: "https://github.com/x/y/pull/125",
status: "open" as const,
headBranch: "fusion/fn-9105",
baseBranch: "main",
};
const github = {
findPrForBranch: vi.fn(),
createPr: vi.fn(),
getPrMergeStatus: vi
.fn()
.mockResolvedValueOnce({
prInfo: openPr,
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
})
.mockResolvedValueOnce({
prInfo: openPr,
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
}),
mergePr: vi.fn(async () => {
throw mergeError;
}),
};
await expect(
processPullRequestMergeTask(
store as never,
"/repo",
task.id,
github as never,
() => undefined,
),
).rejects.toThrow(mergeError.message);
expect(github.mergePr).toHaveBeenCalledWith({ number: 125, method: "squash" });
expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2);
expect(store.updatePrInfo).not.toHaveBeenCalledWith("FN-9105", expect.objectContaining({ status: "merged" }));
expect(store.moveTask).not.toHaveBeenCalled();
});
it("rethrows the original merge error when the post-failure refresh also fails", async () => {
const task: MockTask = {
id: "FN-9106",
title: "test",
description: "desc",
column: "in-review",
prInfo: {
number: 126,
url: "https://github.com/x/y/pull/126",
status: "open",
headBranch: "fusion/fn-9106",
baseBranch: "main",
},
};
const store = makeStore(task);
const mergeError = new Error("merge command failed");
const openPr = {
number: 126,
url: "https://github.com/x/y/pull/126",
status: "open" as const,
headBranch: "fusion/fn-9106",
baseBranch: "main",
};
const github = {
findPrForBranch: vi.fn(),
createPr: vi.fn(),
getPrMergeStatus: vi
.fn()
.mockResolvedValueOnce({
prInfo: openPr,
reviewDecision: "APPROVED",
checks: [],
mergeReady: true,
blockingReasons: [],
})
.mockRejectedValueOnce(new Error("status refresh failed")),
mergePr: vi.fn(async () => {
throw mergeError;
}),
};
await expect(
processPullRequestMergeTask(
store as never,
"/repo",
task.id,
github as never,
() => undefined,
),
).rejects.toThrow(mergeError.message);
expect(github.mergePr).toHaveBeenCalledWith({ number: 126, method: "squash" });
expect(github.getPrMergeStatus).toHaveBeenCalledTimes(2);
expect(store.moveTask).not.toHaveBeenCalled();
});
it("preserves PR number/url through create, refresh, and merge completion", async () => {
const task: MockTask = {
id: "FN-9103",

34
packages/cli/src/commands/task-lifecycle.ts
View File

@@ -175,11 +175,12 @@ async function finalizePullRequestMerge(
cwd: string,
task: TaskDetail,
prInfo: PrInfo,
message = "Pull request merged",
): Promise<void> {
await cleanupMergedTaskArtifacts(cwd, task);
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
await store.moveTask(task.id, "done");
await store.logEntry(task.id, "Pull request merged", `PR #${prInfo.number}: ${prInfo.url}`);
await store.logEntry(task.id, message, `PR #${prInfo.number}: ${prInfo.url}`);
}
/**
@@ -316,7 +317,36 @@ export async function processPullRequestMergeTask(
return "waiting";
}
await store.updateTask(task.id, { status: "merging-pr" });
const mergedPr = await github.mergePr({ number: prInfo.number, method: "squash" });
let mergedPr: PrInfo;
try {
mergedPr = await github.mergePr({ number: prInfo.number, method: "squash" });
} catch (err: unknown) {
let refreshedStatus: Awaited<ReturnType<GitHubOperations["getPrMergeStatus"]>>;
try {
refreshedStatus = await github.getPrMergeStatus(mergeTarget.branch, branch, prInfo.number);
} catch {
throw err;
}
const refreshedAfterFailure: PrInfo = {
...prInfo,
...refreshedStatus.prInfo,
lastCheckedAt: new Date().toISOString(),
};
await store.updatePrInfo(task.id, refreshedAfterFailure);
if (refreshedAfterFailure.status === "merged") {
await finalizePullRequestMerge(
store,
cwd,
task,
refreshedAfterFailure,
"Pull request already merged after merge command failed; reconciled task state from GitHub",
);
return "merged";
}
throw err;
}
await store.updatePrInfo(task.id, { ...mergedPr, lastCheckedAt: new Date().toISOString() });
await finalizePullRequestMerge(store, cwd, task, mergedPr);
return "merged";