diff --git a/src/app/services/api.ts b/src/app/services/api.ts index b60070c7..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 = "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, 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 c2f2b347..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 -> 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" }, })); @@ -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", () => { 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/**"],