fix: prevent merged PRs from showing "Merge conflicts" status
GitHub returns `mergeable: null` for merged/closed PRs. The SCM plugin was misinterpreting this as unknown/problematic status, leading to false "Merge conflicts" warnings in the dashboard. Fixed in two layers: 1. SCM plugin: Check PR state first in getMergeability(). Return clean result immediately for merged/closed PRs without querying mergeable. 2. Dashboard: Skip merge conflict display for non-open PRs in SessionDetail component. SessionCard already had protection via early return in getAlerts(). Closes: bug described in task description Tests: Added tests for merged/closed PR cases, all 54 tests passing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
1cb52c122d
commit
e0f4a9dc27
|
|
@ -506,6 +506,20 @@ function createGitHubSCM(): SCM {
|
|||
async getMergeability(pr: PRInfo): Promise<MergeReadiness> {
|
||||
const blockers: string[] = [];
|
||||
|
||||
// First, check if the PR is merged or closed
|
||||
// GitHub returns mergeable=null for merged/closed PRs, which is not useful
|
||||
const state = await this.getPRState(pr);
|
||||
if (state === "merged" || state === "closed") {
|
||||
// For merged/closed PRs, return a clean result without querying mergeable status
|
||||
return {
|
||||
mergeable: true,
|
||||
ciPassing: true,
|
||||
approved: true,
|
||||
noConflicts: true,
|
||||
blockers: [],
|
||||
};
|
||||
}
|
||||
|
||||
// Fetch PR details with merge state
|
||||
const raw = await gh([
|
||||
"pr",
|
||||
|
|
|
|||
|
|
@ -536,7 +536,41 @@ describe("scm-github plugin", () => {
|
|||
// ---- getMergeability ---------------------------------------------------
|
||||
|
||||
describe("getMergeability", () => {
|
||||
it("returns clean result for merged PRs without querying mergeable status", async () => {
|
||||
// getPRState call
|
||||
mockGh({ state: "MERGED" });
|
||||
|
||||
const result = await scm.getMergeability(pr);
|
||||
expect(result).toEqual({
|
||||
mergeable: true,
|
||||
ciPassing: true,
|
||||
approved: true,
|
||||
noConflicts: true,
|
||||
blockers: [],
|
||||
});
|
||||
// Should only call gh once (for getPRState), not for mergeable/CI
|
||||
expect(ghMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns clean result for closed PRs without querying mergeable status", async () => {
|
||||
// getPRState call
|
||||
mockGh({ state: "CLOSED" });
|
||||
|
||||
const result = await scm.getMergeability(pr);
|
||||
expect(result).toEqual({
|
||||
mergeable: true,
|
||||
ciPassing: true,
|
||||
approved: true,
|
||||
noConflicts: true,
|
||||
blockers: [],
|
||||
});
|
||||
// Should only call gh once (for getPRState), not for mergeable/CI
|
||||
expect(ghMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns mergeable when everything is clear", async () => {
|
||||
// getPRState call (for open PR)
|
||||
mockGh({ state: "OPEN" });
|
||||
// PR view
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "CLEAN", isDraft: false });
|
||||
// CI checks (called by getCISummary)
|
||||
|
|
@ -553,6 +587,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports CI failures as blockers", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "UNSTABLE", isDraft: false });
|
||||
mockGh([{ name: "build", state: "FAILURE" }]);
|
||||
|
||||
|
|
@ -564,6 +599,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports UNSTABLE merge state even when CI fetch fails", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "UNSTABLE", isDraft: false });
|
||||
mockGhError("rate limited");
|
||||
|
||||
|
|
@ -575,6 +611,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports changes requested as blockers", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "CHANGES_REQUESTED", mergeStateStatus: "CLEAN", isDraft: false });
|
||||
mockGh([]); // no CI checks
|
||||
|
||||
|
|
@ -584,6 +621,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports review required as blocker", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "REVIEW_REQUIRED", mergeStateStatus: "BLOCKED", isDraft: false });
|
||||
mockGh([]);
|
||||
|
||||
|
|
@ -592,6 +630,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports merge conflicts as blockers", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "CONFLICTING", reviewDecision: "APPROVED", mergeStateStatus: "DIRTY", isDraft: false });
|
||||
mockGh([]);
|
||||
|
||||
|
|
@ -601,6 +640,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports UNKNOWN mergeable as noConflicts false", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "UNKNOWN", reviewDecision: "APPROVED", mergeStateStatus: "CLEAN", isDraft: false });
|
||||
mockGh([{ name: "build", state: "SUCCESS" }]);
|
||||
|
||||
|
|
@ -611,6 +651,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports draft status as blocker", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "MERGEABLE", reviewDecision: "APPROVED", mergeStateStatus: "DRAFT", isDraft: true });
|
||||
mockGh([{ name: "build", state: "SUCCESS" }]);
|
||||
|
||||
|
|
@ -620,6 +661,7 @@ describe("scm-github plugin", () => {
|
|||
});
|
||||
|
||||
it("reports multiple blockers simultaneously", async () => {
|
||||
mockGh({ state: "OPEN" }); // getPRState
|
||||
mockGh({ mergeable: "CONFLICTING", reviewDecision: "CHANGES_REQUESTED", mergeStateStatus: "DIRTY", isDraft: true });
|
||||
mockGh([{ name: "build", state: "FAILURE" }]);
|
||||
|
||||
|
|
|
|||
|
|
@ -450,7 +450,8 @@ function IssuesList({ pr }: { pr: DashboardPR }) {
|
|||
});
|
||||
}
|
||||
|
||||
if (!pr.mergeability.noConflicts) {
|
||||
// Only show merge conflicts for open PRs (merged/closed PRs don't have mergeable status)
|
||||
if (pr.state === "open" && !pr.mergeability.noConflicts) {
|
||||
issues.push({
|
||||
icon: "\u2717",
|
||||
color: "var(--color-accent-red)",
|
||||
|
|
|
|||
Loading…
Reference in New Issue