From 806361ec1646ef31e1232f984849f43db903245f Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 7 Apr 2026 15:38:01 -0400 Subject: [PATCH 1/3] 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. --- src/app/services/api.ts | 6 +++--- tests/services/hot-poll.test.ts | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/app/services/api.ts b/src/app/services/api.ts index b60070c7..bb74f42e 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1284,7 +1284,7 @@ export async function fetchPREnrichment( const mss = node.mergeStateStatus; if (mss === "DIRTY" || mss === "BEHIND") { checkStatus = "conflict"; - } else if (mss === "UNSTABLE") { + } else if (mss === "UNSTABLE" && checkStatus !== "pending") { checkStatus = "failure"; } @@ -1670,7 +1670,7 @@ async function graphqlSearchPRs( const mss = node.mergeStateStatus; if (mss === "DIRTY" || mss === "BEHIND") { checkStatus = "conflict"; - } else if (mss === "UNSTABLE") { + } else if (mss === "UNSTABLE" && checkStatus !== "pending") { checkStatus = "failure"; } else if (mss === "UNKNOWN" && checkStatus === null) { checkStatus = null; // no-op, kept explicit for clarity @@ -2105,7 +2105,7 @@ export async function fetchHotPRStatus( const mss = node.mergeStateStatus; if (mss === "DIRTY" || mss === "BEHIND") { checkStatus = "conflict"; - } else if (mss === "UNSTABLE") { + } else if (mss === "UNSTABLE" && checkStatus !== "pending") { checkStatus = "failure"; } diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index c2f2b347..2ae1c837 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -134,14 +134,14 @@ describe("fetchHotPRStatus", () => { expect(results.get(43)!.checkStatus).toBe("conflict"); }); - it("applies mergeStateStatus overrides: UNSTABLE -> failure", async () => { + it("applies mergeStateStatus overrides: UNSTABLE with failed rollup -> 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: "FAILURE" } } }] }, }], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, })); @@ -149,6 +149,22 @@ describe("fetchHotPRStatus", () => { 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", () => { From c070d46ff1283e125a5ab27c104c50c333ece4ae Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 7 Apr 2026 15:45:59 -0400 Subject: [PATCH 2/3] fix(test): increases hookTimeout for browser test project --- vitest.workspace.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/vitest.workspace.ts b/vitest.workspace.ts index 84898d06..56881a0f 100644 --- a/vitest.workspace.ts +++ b/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/**"], From 1d8909e9dc8ceff2a68562b0558dc35856f6d31d Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 7 Apr 2026 16:36:57 -0400 Subject: [PATCH 3/3] refactor(api): extracts applyMergeStateOverride helper --- src/app/services/api.ts | 50 ++++++------ tests/services/api-optimization.test.ts | 103 ++++++++++++++++++++++++ tests/services/hot-poll.test.ts | 4 +- 3 files changed, 132 insertions(+), 25 deletions(-) diff --git a/src/app/services/api.ts b/src/app/services/api.ts index bb74f42e..718bda18 100644 --- a/src/app/services/api.ts +++ b/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, diff --git a/tests/services/api-optimization.test.ts b/tests/services/api-optimization.test.ts index 1ba11fcd..d30d1faa 100644 --- a/tests/services/api-optimization.test.ts +++ b/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"); + }); +}); diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 2ae1c837..777aad34 100644 --- a/tests/services/hot-poll.test.ts +++ b/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" }, }));