fix(api): preserves pending check status when UNSTABLE (#56)

* fix(api): preserves pending status when UNSTABLE

GitHub's mergeStateStatus UNSTABLE means "mergeable with non-passing
commit status", which includes both failing AND pending checks. The
UNSTABLE override was unconditionally setting checkStatus to "failure",
clobbering the correct "pending" state from statusCheckRollup.

Guards the override so it only applies when rollup is not pending.

* fix(test): increases hookTimeout for browser test project

* 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 = "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 = "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 = "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,21 +134,37 @@ describe("fetchHotPRStatus", () => {
     expect(results.get(43)!.checkStatus).toBe("conflict");
   });
 
-  it("applies mergeStateStatus overrides: UNSTABLE -> 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: "PENDING" } } }] },
+        commits: { nodes: [{ commit: { statusCheckRollup: { state: "SUCCESS" } } }] },
       }],
       rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" },
     }));
 
     const { results } = await fetchHotPRStatus(octokit as never, ["PR_node3"]);
     expect(results.get(44)!.checkStatus).toBe("failure");
   });
+
+  it("preserves pending when mergeStateStatus is UNSTABLE but rollup is PENDING", async () => {
+    const octokit = makeOctokit(undefined, () => Promise.resolve({
+      nodes: [{
+        databaseId: 45,
+        state: "OPEN",
+        mergeStateStatus: "UNSTABLE",
+        reviewDecision: null,
+        commits: { nodes: [{ commit: { statusCheckRollup: { state: "PENDING" } } }] },
+      }],
+      rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" },
+    }));
+
+    const { results } = await fetchHotPRStatus(octokit as never, ["PR_node4"]);
+    expect(results.get(45)!.checkStatus).toBe("pending");
+  });
 });
 
 describe("fetchWorkflowRunById", () => {

vitest.workspace.ts

@@ -13,6 +13,7 @@ export default defineConfig({
           name: "browser",
           environment: "happy-dom",
           globals: true,
+          hookTimeout: 30_000,
           setupFiles: ["tests/setup.ts"],
           include: ["tests/**/*.test.ts", "tests/**/*.test.tsx"],
           exclude: ["tests/worker/**"],