fix: prevent merged PRs from showing 'Merge conflicts' status (#60)
* 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> * fix: only skip mergeability checks for merged PRs, not closed PRs Addresses review comment: A closed-but-unmerged PR should still have its mergeability checked accurately. Only merged PRs should skip the checks since they're already merged. Changes: - SCM plugin: Only return clean status for state === "merged" - Dashboard: Show conflicts for closed PRs (pr.state !== "merged") - Tests: Updated to verify closed PRs still get checked Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
66005c05c5
commit
2fce2c70f0
|
|
@ -506,6 +506,21 @@ function createGitHubSCM(): SCM {
|
|||
async getMergeability(pr: PRInfo): Promise<MergeReadiness> {
|
||||
const blockers: string[] = [];
|
||||
|
||||
// First, check if the PR is merged
|
||||
// GitHub returns mergeable=null for merged PRs, which is not useful
|
||||
// Note: We only skip checks for merged PRs. Closed PRs still need accurate status.
|
||||
const state = await this.getPRState(pr);
|
||||
if (state === "merged") {
|
||||
// For merged 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,39 @@ 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("still checks mergeability for closed PRs (not merged)", async () => {
|
||||
// getPRState call
|
||||
mockGh({ state: "CLOSED" });
|
||||
// PR view (closed PRs still get checked)
|
||||
mockGh({ mergeable: "CONFLICTING", reviewDecision: "APPROVED", mergeStateStatus: "DIRTY", isDraft: false });
|
||||
// CI checks
|
||||
mockGh([]);
|
||||
|
||||
const result = await scm.getMergeability(pr);
|
||||
expect(result.noConflicts).toBe(false);
|
||||
expect(result.blockers).toContain("Merge conflicts");
|
||||
// Closed PRs go through normal checks, unlike merged PRs
|
||||
});
|
||||
|
||||
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 +585,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 +597,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 +609,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 +619,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 +628,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 +638,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 +649,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 +659,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/closed PRs (merged PRs don't have meaningful mergeable status)
|
||||
if (pr.state !== "merged" && !pr.mergeability.noConflicts) {
|
||||
issues.push({
|
||||
icon: "\u2717",
|
||||
color: "var(--color-accent-red)",
|
||||
|
|
|
|||
Loading…
Reference in New Issue