Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 27 additions & 23 deletions src/app/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
103 changes: 103 additions & 0 deletions tests/services/api-optimization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
fetchPullRequests,
fetchIssuesAndPullRequests,
fetchWorkflowRuns,
fetchPREnrichment,
type RepoRef,
} from "../../src/app/services/api";
import { clearCache } from "../../src/app/stores/cache";
Expand Down Expand Up @@ -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");
});
});
20 changes: 18 additions & 2 deletions tests/services/hot-poll.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
1 change: 1 addition & 0 deletions vitest.workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/**"],
Expand Down