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

FN-8835: block unmergeable pull requests

Browse Source 
Fail closed before auto-merging pull requests that GitHub reports as unmergeable.

- Require normalized clean mergeability in PR readiness checks.
- Cover protected, behind, conflicting, and unknown PR states across CLI and dashboard tests.
- Add a patch changeset for the corrected auto-merge behavior.

Files changed:
 .changeset/fn-8835-pr-merge-readiness.md           |  7 +++
 .../src/commands/__tests__/task-lifecycle.test.ts  | 21 ++++---
 packages/dashboard/src/__tests__/github.test.ts    | 64 ++++++++++++++++++----
 packages/dashboard/src/github.ts                   | 11 ++++
 4 files changed, 84 insertions(+), 19 deletions(-)

Fusion-Task-Id: FN-8835

Fusion-Task-Lineage: 698dacf9-5f24-42b7-b927-ae5b5579ee3f

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-08-08 18:13:52 -07:00
parent e2522ebbb0
commit 54d1ccb8d5
4 changed files with 84 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-8835-pr-merge-readiness.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Prevent auto-merge attempts for branch-protected, behind, conflicting, or unknown PR states.
category: fix
dev: The legacy PR merge gate now requires normalized mergeability to be `clean` while preserving optional approval and check policy.

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

@@ -517,7 +517,7 @@ describe("processPullRequestMergeTask", () => {
expect(github.createPr).toHaveBeenCalledWith(expect.objectContaining({ head: getTaskBranchName(task.id) }));
});
it("does not create duplicate group PR when branch-group PR already exists", async () => {
it("holds a branch-protected shared-group PR without calling mergePr", async () => {
const task: MockTask = {
id: "FN-9012",
title: "group member",
@@ -553,11 +553,11 @@ describe("processPullRequestMergeTask", () => {
findPrForBranch: vi.fn(async () => null),
createPr: vi.fn(),
getPrMergeStatus: vi.fn(async () => ({
prInfo: { number: 22, status: "open" as const, url: "https://github.com/x/y/pull/22" },
reviewDecision: null,
prInfo: { number: 22, status: "open" as const, url: "https://github.com/x/y/pull/22", mergeable: "blocked" as const },
reviewDecision: "REVIEW_REQUIRED",
checks: [],
mergeReady: false,
blockingReasons: [],
blockingReasons: ["PR mergeability is blocked"],
})),
mergePr: vi.fn(),
};
@@ -566,6 +566,8 @@ describe("processPullRequestMergeTask", () => {
expect(github.createPr).not.toHaveBeenCalled();
expect(github.getPrMergeStatus).toHaveBeenCalledWith("owner", "repo", 22);
expect(github.mergePr).not.toHaveBeenCalled();
expect(store.updateTask).toHaveBeenCalledWith(task.id, { status: "awaiting-pr-checks" });
expect(store.updateBranchGroup).toHaveBeenCalledWith("BG-2", expect.objectContaining({
prNumber: 22,
prUrl: "https://github.com/x/y/pull/22",
@@ -845,7 +847,7 @@ describe("processPullRequestMergeTask", () => {
});
});
it("does not bump mergeRetries for a non-conflicting not-ready PR", async () => {
it("holds a branch-protected review-required PR without a merge attempt or conflict retry", async () => {
const task: MockTask = {
id: "FN-9021",
title: "test",
@@ -862,11 +864,11 @@ describe("processPullRequestMergeTask", () => {
findPrForBranch: vi.fn(async () => existingPr),
createPr: vi.fn(),
getPrMergeStatus: vi.fn(async () => ({
prInfo: { ...existingPr, mergeable: "unknown" as const },
reviewDecision: null,
prInfo: { ...existingPr, mergeable: "blocked" as const },
reviewDecision: "REVIEW_REQUIRED",
checks: [],
mergeReady: false,
blockingReasons: [],
blockingReasons: ["PR mergeability is blocked"],
})),
mergePr: vi.fn(),
};
@@ -875,6 +877,7 @@ describe("processPullRequestMergeTask", () => {
expect(result).toBe("waiting");
expect(store.updateTask).toHaveBeenCalledWith(task.id, { status: "awaiting-pr-checks" });
expect(github.mergePr).not.toHaveBeenCalled();
});
it("skips the push when an existing PR already covers the branch", async () => {
@@ -1456,7 +1459,7 @@ describe("processPullRequestMergeTask", () => {
// checks and no blocking review state, so isPrMergeReady returns
// mergeReady: true. Without the gate this would auto-merge.
return {
prInfo,
prInfo: { ...prInfo, mergeable: "clean" as const },
reviewDecision,
checks: [],
mergeReady: true,

64
packages/dashboard/src/__tests__/github.test.ts
View File

@@ -1665,6 +1665,8 @@ describe("GitHubClient", () => {
const result = await client.getPrMergeStatus("owner", "repo", 42);
expect(result.mergeable).toBe("conflicting");
expect(result.prInfo.mergeable).toBe("conflicting");
expect(result.mergeReady).toBe(false);
expect(result.blockingReasons).toEqual(["PR mergeability is conflicting"]);
});
it("maps BEHIND merge-state to behind", async () => {
@@ -1684,6 +1686,8 @@ describe("GitHubClient", () => {
const result = await client.getPrMergeStatus("owner", "repo", 42);
expect(result.mergeable).toBe("behind");
expect(result.prInfo.mergeable).toBe("behind");
expect(result.mergeReady).toBe(false);
expect(result.blockingReasons).toEqual(["PR mergeability is behind"]);
});
it("maps BLOCKED merge-state to blocked", async () => {
@@ -1693,7 +1697,8 @@ describe("GitHubClient", () => {
url: "https://github.com/owner/repo/pull/42",
title: "Blocked PR",
state: "OPEN",
reviewDecision: "APPROVED",
reviewDecision: "REVIEW_REQUIRED",
mergeable: "MERGEABLE",
mergeStateStatus: "BLOCKED",
baseRefName: "main",
headRefName: "fusion/fn-093",
@@ -1703,6 +1708,8 @@ describe("GitHubClient", () => {
const result = await client.getPrMergeStatus("owner", "repo", 42);
expect(result.mergeable).toBe("blocked");
expect(result.prInfo.mergeable).toBe("blocked");
expect(result.mergeReady).toBe(false);
expect(result.blockingReasons).toEqual(["PR mergeability is blocked"]);
});
it("maps missing mergeability fields to unknown", async () => {
@@ -1721,9 +1728,11 @@ describe("GitHubClient", () => {
const result = await client.getPrMergeStatus("owner", "repo", 42);
expect(result.mergeable).toBe("unknown");
expect(result.prInfo.mergeable).toBe("unknown");
expect(result.mergeReady).toBe(false);
expect(result.blockingReasons).toEqual(["PR mergeability is unknown"]);
});
it("falls back to GraphQL API when gh CLI merge-status lookup fails and token is available", async () => {
it("fails closed through the GraphQL API when branch protection blocks a review-required PR", async () => {
mockRunGhJsonAsync.mockRejectedValue(new Error("gh failed"));
const clientWithToken = new GitHubClient("ghp_token");
const mockFetch = vi.fn().mockResolvedValue({
@@ -1734,11 +1743,11 @@ describe("GitHubClient", () => {
pullRequest: {
number: 42,
url: "https://github.com/owner/repo/pull/42",
title: "Fallback PR",
title: "Branch-protected PR",
state: "OPEN",
reviewDecision: null,
mergeable: "CONFLICTING",
mergeStateStatus: "DIRTY",
reviewDecision: "REVIEW_REQUIRED",
mergeable: "MERGEABLE",
mergeStateStatus: "BLOCKED",
baseRefName: "main",
headRefName: "fusion/fn-093",
comments: { totalCount: 0 },
@@ -1790,9 +1799,10 @@ describe("GitHubClient", () => {
const result = await clientWithToken.getPrMergeStatus("owner", "repo", 42);
expect(result.mergeReady).toBe(true);
expect(result.mergeable).toBe("conflicting");
expect(result.prInfo.mergeable).toBe("conflicting");
expect(result.mergeReady).toBe(false);
expect(result.mergeable).toBe("blocked");
expect(result.prInfo.mergeable).toBe("blocked");
expect(result.blockingReasons).toEqual(["PR mergeability is blocked"]);
expect(result.checks).toEqual([
{
name: "ci",
@@ -2140,7 +2150,7 @@ describe("GitHubClient", () => {
describe("isPrMergeReady", () => {
it("blocks closed PRs", () => {
expect(isPrMergeReady({ status: "closed", reviewDecision: null, checks: [] })).toEqual({
expect(isPrMergeReady({ status: "closed", reviewDecision: null, checks: [], mergeable: "clean" })).toEqual({
ready: false,
blockingReasons: ["PR is closed"],
});
@@ -2151,6 +2161,7 @@ describe("GitHubClient", () => {
status: "open",
reviewDecision: "CHANGES_REQUESTED",
checks: [{ name: "ci", required: true, state: "success" }],
mergeable: "clean",
})).toEqual({
ready: false,
blockingReasons: ["changes requested review is active"],
@@ -2162,6 +2173,7 @@ describe("GitHubClient", () => {
status: "open",
reviewDecision: null,
checks: [{ name: "ci", required: true, state: "pending" }],
mergeable: "clean",
})).toEqual({
ready: false,
blockingReasons: ["required checks not successful: ci (pending)"],
@@ -2176,8 +2188,40 @@ describe("GitHubClient", () => {
{ name: "required-ci", required: true, state: "success" },
{ name: "optional-preview", required: false, state: "failure" },
],
mergeable: "clean",
})).toEqual({ ready: true, blockingReasons: [] });
});
it.each(["blocked", "behind", "conflicting", "unknown"] as const)(
"fails closed when mergeability is %s",
(mergeable) => {
expect(isPrMergeReady({
status: "open",
reviewDecision: "APPROVED",
checks: [{ name: "ci", required: true, state: "success" }],
mergeable,
})).toEqual({
ready: false,
blockingReasons: [`PR mergeability is ${mergeable}`],
});
},
);
it("aggregates branch protection with existing review and required-check blockers", () => {
expect(isPrMergeReady({
status: "open",
reviewDecision: "CHANGES_REQUESTED",
checks: [{ name: "ci", required: true, state: "failure" }],
mergeable: "blocked",
})).toEqual({
ready: false,
blockingReasons: [
"changes requested review is active",
"PR mergeability is blocked",
"required checks not successful: ci (failure)",
],
});
});
});
describe("error handling when gh CLI not available", () => {

11
packages/dashboard/src/github.ts
View File

@@ -735,10 +735,15 @@ function toPrInfo(input: {
};
}
/*
FNXC:PrMergeReadiness 2026-08-09-00:48:
FN-8835 requires the live pull-request merge gate to fail closed on GitHub's normalized mergeability state. Branch-protection BLOCKED, stale BEHIND, conflicts, and unknown state must wait before any merge request; legacy approval and optional-check policy remain unchanged.
*/
export function isPrMergeReady(input: {
status: PrInfo["status"];
reviewDecision: ReviewDecision;
checks: PrCheckStatus[];
mergeable: PrConflictState;
}): { ready: boolean; blockingReasons: string[] } {
const blockingReasons: string[] = [];
@@ -750,6 +755,10 @@ export function isPrMergeReady(input: {
blockingReasons.push("changes requested review is active");
}
if (input.mergeable !== "clean") {
blockingReasons.push(`PR mergeability is ${input.mergeable}`);
}
const blockingChecks = input.checks.filter(
(check) => check.required && check.state !== "success",
);
@@ -1816,6 +1825,7 @@ export class GitHubClient {
status: prInfo.status,
reviewDecision: pr.reviewDecision ?? null,
checks: normalizedChecks,
mergeable,
});
return {
@@ -1975,6 +1985,7 @@ export class GitHubClient {
status: prInfo.status,
reviewDecision: pr.reviewDecision,
checks,
mergeable,
});
return {