refactor(api): extracts applyMergeStateOverride helper

Author: wgordon17

src/app/services/api.ts

@@ -1280,13 +1280,10 @@ export async function fetchPREnrichment(
           .filter((l): l is string => l != null);
         const reviewerLogins = [...new Set([...pendingLogins, ...actualLogins].map(l => l.toLowerCase()))];
 
-        let checkStatus = mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null);
-        const mss = node.mergeStateStatus;
-        if (mss === "DIRTY" || mss === "BEHIND") {
-          checkStatus = "conflict";
-        } else if (mss === "UNSTABLE" && checkStatus !== "pending") {
-          checkStatus = "failure";
-        }
+        const checkStatus = applyMergeStateOverride(
+          node.mergeStateStatus,
+          mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null),
+        );
 
         let headRepository: PREnrichmentData["headRepository"] = null;
         if (node.headRepository) {
@@ -1619,6 +1616,21 @@ function mapCheckStatus(state: string | null | undefined): CheckStatus["status"]
   return null;
 }
 
+/**
+ * Applies mergeStateStatus overrides to a PR's checkStatus.
+ * DIRTY/BEHIND → conflict. UNSTABLE → failure unless checkStatus is already pending
+ * (a pending rollup means checks are still running, not conclusively failed).
+ * All other values (CLEAN, BLOCKED, HAS_HOOKS, UNKNOWN) pass through unchanged.
+ */
+function applyMergeStateOverride(
+  mergeStateStatus: string | null | undefined,
+  checkStatus: CheckStatus["status"],
+): CheckStatus["status"] {
+  if (mergeStateStatus === "DIRTY" || mergeStateStatus === "BEHIND") return "conflict";
+  if (mergeStateStatus === "UNSTABLE" && checkStatus !== "pending") return "failure";
+  return checkStatus;
+}
+
 /**
  * Maps a GraphQL reviewDecision string to the typed union or null.
  */
@@ -1664,17 +1676,12 @@ async function graphqlSearchPRs(
       .filter((l): l is string => l != null);
     const reviewerLogins = [...new Set([...pendingLogins, ...actualLogins].map(l => l.toLowerCase()))];
 
-    let checkStatus = mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null);
     // mergeStateStatus overrides checkStatus when it indicates action is needed.
     // BLOCKED means required checks/reviews haven't passed — leave checkStatus from rollup.
-    const mss = node.mergeStateStatus;
-    if (mss === "DIRTY" || mss === "BEHIND") {
-      checkStatus = "conflict";
-    } else if (mss === "UNSTABLE" && checkStatus !== "pending") {
-      checkStatus = "failure";
-    } else if (mss === "UNKNOWN" && checkStatus === null) {
-      checkStatus = null; // no-op, kept explicit for clarity
-    }
+    const checkStatus = applyMergeStateOverride(
+      node.mergeStateStatus,
+      mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null),
+    );
 
     // Store fork info for fallback detection
     if (node.headRepository) {
@@ -2101,13 +2108,10 @@ export async function fetchHotPRStatus(
     for (const node of response.nodes) {
       if (!node || node.databaseId == null) continue;
 
-      let checkStatus = mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null);
-      const mss = node.mergeStateStatus;
-      if (mss === "DIRTY" || mss === "BEHIND") {
-        checkStatus = "conflict";
-      } else if (mss === "UNSTABLE" && checkStatus !== "pending") {
-        checkStatus = "failure";
-      }
+      const checkStatus = applyMergeStateOverride(
+        node.mergeStateStatus,
+        mapCheckStatus(node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null),
+      );
 
       results.set(node.databaseId, {
         state: node.state,

tests/services/api-optimization.test.ts

@@ -5,6 +5,7 @@ import {
   fetchPullRequests,
   fetchIssuesAndPullRequests,
   fetchWorkflowRuns,
+  fetchPREnrichment,
   type RepoRef,
 } from "../../src/app/services/api";
 import { clearCache } from "../../src/app/stores/cache";
@@ -676,3 +677,105 @@ describe("scaling behavior", () => {
     });
   }
 });
+
+// ── fetchPREnrichment: UNSTABLE+pending override ───────────────────────────────
+
+describe("fetchPREnrichment mergeStateStatus UNSTABLE override", () => {
+  function makeEnrichmentOctokit(mergeStateStatus: string, rollupState: string | null) {
+    return {
+      request: vi.fn(async () => ({ data: [], headers: {} })),
+      graphql: vi.fn(async () => ({
+        nodes: [{
+          databaseId: 100,
+          headRefOid: "abc123",
+          headRepository: { owner: { login: "octocat" }, nameWithOwner: "octocat/Hello-World" },
+          mergeStateStatus,
+          assignees: { nodes: [] },
+          reviewRequests: { nodes: [] },
+          latestReviews: { totalCount: 0, nodes: [] },
+          additions: 5,
+          deletions: 2,
+          changedFiles: 1,
+          comments: { totalCount: 0 },
+          reviewThreads: { totalCount: 0 },
+          commits: {
+            nodes: rollupState
+              ? [{ commit: { statusCheckRollup: { state: rollupState } } }]
+              : [],
+          },
+        }],
+        rateLimit: { limit: 5000, remaining: 4999, resetAt: new Date(Date.now() + 3600000).toISOString() },
+      })),
+      paginate: { iterator: vi.fn() },
+    };
+  }
+
+  it("preserves pending checkStatus when UNSTABLE and rollup is PENDING", async () => {
+    const octokit = makeEnrichmentOctokit("UNSTABLE", "PENDING");
+    const { enrichments } = await fetchPREnrichment(octokit as never, ["PR_node1"]);
+    expect(enrichments.get(100)!.checkStatus).toBe("pending");
+  });
+
+  it("forces failure checkStatus when UNSTABLE and rollup is SUCCESS", async () => {
+    const octokit = makeEnrichmentOctokit("UNSTABLE", "SUCCESS");
+    const { enrichments } = await fetchPREnrichment(octokit as never, ["PR_node1"]);
+    expect(enrichments.get(100)!.checkStatus).toBe("failure");
+  });
+
+  it("forces failure checkStatus when UNSTABLE and rollup is absent", async () => {
+    const octokit = makeEnrichmentOctokit("UNSTABLE", null);
+    const { enrichments } = await fetchPREnrichment(octokit as never, ["PR_node1"]);
+    expect(enrichments.get(100)!.checkStatus).toBe("failure");
+  });
+});
+
+// ── fetchPullRequests (graphqlSearchPRs): UNSTABLE+pending override ───────────
+
+describe("fetchPullRequests processPRNode UNSTABLE override", () => {
+  const repos: RepoRef[] = [{ owner: "octocat", name: "Hello-World", fullName: "octocat/Hello-World" }];
+
+  function makePRNodeWithMSS(mergeStateStatus: string, rollupState: string | null) {
+    return {
+      ...graphqlPRNode,
+      mergeStateStatus,
+      commits: {
+        nodes: rollupState
+          ? [{ commit: { statusCheckRollup: { state: rollupState } } }]
+          : [],
+      },
+    };
+  }
+
+  it("preserves pending checkStatus when UNSTABLE and rollup is PENDING", async () => {
+    const octokit = makeOctokit(async () => ({
+      search: makeSearchResponse([makePRNodeWithMSS("UNSTABLE", "PENDING")]),
+      rateLimit,
+    }));
+
+    const result = await fetchPullRequests(octokit as unknown as OctokitLike, repos, "testuser");
+    expect(result.pullRequests).toHaveLength(1);
+    expect(result.pullRequests[0].checkStatus).toBe("pending");
+  });
+
+  it("forces failure checkStatus when UNSTABLE and rollup is SUCCESS", async () => {
+    const octokit = makeOctokit(async () => ({
+      search: makeSearchResponse([makePRNodeWithMSS("UNSTABLE", "SUCCESS")]),
+      rateLimit,
+    }));
+
+    const result = await fetchPullRequests(octokit as unknown as OctokitLike, repos, "testuser");
+    expect(result.pullRequests).toHaveLength(1);
+    expect(result.pullRequests[0].checkStatus).toBe("failure");
+  });
+
+  it("forces failure checkStatus when UNSTABLE and rollup is absent", async () => {
+    const octokit = makeOctokit(async () => ({
+      search: makeSearchResponse([makePRNodeWithMSS("UNSTABLE", null)]),
+      rateLimit,
+    }));
+
+    const result = await fetchPullRequests(octokit as unknown as OctokitLike, repos, "testuser");
+    expect(result.pullRequests).toHaveLength(1);
+    expect(result.pullRequests[0].checkStatus).toBe("failure");
+  });
+});

tests/services/hot-poll.test.ts

@@ -134,14 +134,14 @@ describe("fetchHotPRStatus", () => {
     expect(results.get(43)!.checkStatus).toBe("conflict");
   });
 
-  it("applies mergeStateStatus overrides: UNSTABLE with failed rollup -> failure", async () => {
+  it("applies mergeStateStatus overrides: UNSTABLE overrides SUCCESS rollup to failure", async () => {
     const octokit = makeOctokit(undefined, () => Promise.resolve({
       nodes: [{
         databaseId: 44,
         state: "OPEN",
         mergeStateStatus: "UNSTABLE",
         reviewDecision: null,
-        commits: { nodes: [{ commit: { statusCheckRollup: { state: "FAILURE" } } }] },
+        commits: { nodes: [{ commit: { statusCheckRollup: { state: "SUCCESS" } } }] },
       }],
       rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" },
     }));