diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index e18643aa0..94566d37a 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -1477,3 +1477,198 @@ describe("getStates", () => { expect(lm.getStates().get("app-1")).toBe("working"); }); }); + +describe("rate limiting optimizations", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + // PR with owner/repo that matches the test config's "org/my-app" + function makeMatchingPR() { + return makePR({ owner: "org", repo: "my-app" }); + } + + it("skips getMergeability() when batch enrichment has hasConflicts data", async () => { + config.reactions = { + "merge-conflicts": { + auto: true, + action: "send-to-agent", + message: "Resolve conflicts.", + }, + }; + + const pr = makeMatchingPR(); + const getMergeabilityMock = vi.fn(); + const mockSCM = createMockSCM({ + getMergeability: getMergeabilityMock, + getCISummary: vi.fn().mockResolvedValue("passing"), + enrichSessionsPRBatch: vi.fn().mockResolvedValue( + new Map([ + [ + `${pr.owner}/${pr.repo}#${pr.number}`, + { + state: "open" as const, + ciStatus: "passing" as const, + reviewDecision: "none" as const, + mergeable: false, + hasConflicts: true, + }, + ], + ]), + ), + }); + + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const session = makeSession({ id: "s-1", status: "pr_open", pr }); + vi.mocked(mockSessionManager.list).mockResolvedValue([session]); + vi.mocked(mockSessionManager.send).mockResolvedValue(undefined); + + const lm = createLifecycleManager({ config, registry, sessionManager: mockSessionManager }); + lm.start(60_000); + await vi.advanceTimersByTimeAsync(0); + lm.stop(); + + // getMergeability() should NOT be called — batch enrichment has the data + expect(getMergeabilityMock).not.toHaveBeenCalled(); + // Conflict notification should have been sent + expect(mockSessionManager.send).toHaveBeenCalledWith("s-1", "Resolve conflicts."); + }); + + it("skips getCIChecks() when batch enrichment has ciChecks data", async () => { + config.reactions = { + "ci-failed": { + auto: true, + action: "send-to-agent", + message: "CI failing.", + retries: 3, + escalateAfter: 3, + }, + }; + + const pr = makeMatchingPR(); + const getCIChecksMock = vi.fn(); + const mockSCM = createMockSCM({ + getCIChecks: getCIChecksMock, + getCISummary: vi.fn().mockResolvedValue("failing"), + enrichSessionsPRBatch: vi.fn().mockResolvedValue( + new Map([ + [ + `${pr.owner}/${pr.repo}#${pr.number}`, + { + state: "open" as const, + ciStatus: "failing" as const, + reviewDecision: "none" as const, + mergeable: false, + hasConflicts: false, + ciChecks: [ + { name: "lint", status: "failed" as const, conclusion: "FAILURE", url: "https://example.com/lint" }, + { name: "test", status: "passed" as const, conclusion: "SUCCESS" }, + ], + }, + ], + ]), + ), + }); + + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + // Start with pr_open state so that ci_failed transition happens on first poll + const session = makeSession({ id: "s-2", status: "pr_open", pr }); + vi.mocked(mockSessionManager.list).mockResolvedValue([session]); + vi.mocked(mockSessionManager.send).mockResolvedValue(undefined); + + const lm = createLifecycleManager({ config, registry, sessionManager: mockSessionManager }); + lm.start(60_000); + // First poll: transitions to ci_failed, sends reaction message + await vi.advanceTimersByTimeAsync(0); + + vi.mocked(mockSessionManager.send).mockClear(); + + // Second poll: dispatches detailed CI failure info + await vi.advanceTimersByTimeAsync(60_000); + + // getCIChecks() should NOT be called — batch enrichment has ciChecks + expect(getCIChecksMock).not.toHaveBeenCalled(); + // Detailed message with lint check name/URL should be sent + const calls = vi.mocked(mockSessionManager.send).mock.calls; + const sentMessages = calls.map((c) => c[1] as string); + const detailMessage = sentMessages.find((m) => m.includes("lint")); + expect(detailMessage).toBeDefined(); + expect(detailMessage).toContain("https://example.com/lint"); + // Passing check should not be included + expect(detailMessage).not.toContain("test"); + + lm.stop(); + }); + + it("throttles review backlog API calls to at most once per 2 minutes", async () => { + config.reactions = { + "changes-requested": { + auto: true, + action: "send-to-agent", + message: "Handle review comments.", + }, + }; + + const getPendingMock = vi.fn().mockResolvedValue([ + { + id: "c1", + author: "reviewer", + body: "Please fix this", + path: "src/index.ts", + line: 10, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/1", + }, + ]); + const getAutomatedMock = vi.fn().mockResolvedValue([]); + const mockSCM = createMockSCM({ + getPendingComments: getPendingMock, + getAutomatedComments: getAutomatedMock, + }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + vi.mocked(mockSessionManager.send).mockResolvedValue(undefined); + + const lm = setupCheck("app-1", { + session: makeSession({ status: "pr_open", pr: makePR() }), + registry, + }); + + // First check: API called, dispatch happens + await lm.check("app-1"); + expect(getPendingMock).toHaveBeenCalledTimes(1); + vi.mocked(mockSessionManager.send).mockClear(); + getPendingMock.mockClear(); + + // Second check immediately after: throttled — API NOT called + await lm.check("app-1"); + expect(getPendingMock).not.toHaveBeenCalled(); + expect(mockSessionManager.send).not.toHaveBeenCalled(); + + // Advance time past the 2-minute throttle window + await vi.advanceTimersByTimeAsync(2 * 60 * 1000 + 100); + + // Third check: throttle expired — API called again + await lm.check("app-1"); + expect(getPendingMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index 944617512..8e1841c24 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -208,6 +208,16 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan */ const prEnrichmentCache = new Map(); + /** + * Per-session timestamp of last review backlog API check. + * Used to throttle getPendingComments/getAutomatedComments to at most once per 2 minutes. + * In-memory only — resets on restart (acceptable since it's a rate-limit hint, not state). + */ + const lastReviewBacklogCheckAt = new Map(); + + /** Throttle interval for review backlog API calls (2 minutes). */ + const REVIEW_BACKLOG_THROTTLE_MS = 2 * 60 * 1000; + /** * Populate the PR enrichment cache using batch GraphQL queries. * This is called once per poll cycle to fetch data for all PRs efficiently. @@ -739,6 +749,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan if (TERMINAL_STATUSES.has(newStatus)) { clearReactionTracker(session.id, humanReactionKey); clearReactionTracker(session.id, automatedReactionKey); + lastReviewBacklogCheckAt.delete(session.id); updateSessionMetadata(session, { lastPendingReviewFingerprint: "", lastPendingReviewDispatchHash: "", @@ -750,6 +761,27 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return; } + // Throttle review backlog API calls to at most once per 2 minutes. + // Comments don't change faster than this in practice, and the SCM calls + // (getPendingComments + getAutomatedComments) consume API quota on every poll. + // + // Exception: bypass throttle when a transition reaction just fired for a + // review reaction key. The transitionReaction branch records + // lastPendingReviewDispatchHash, which requires the current fingerprint from + // the API. If we throttle here, that metadata never gets written and the + // next unthrottled poll sees a "new" fingerprint, clears the reaction tracker, + // and fires a duplicate dispatch. + const hasRelevantTransition = + transitionReaction?.key === humanReactionKey || + transitionReaction?.key === automatedReactionKey; + if (!hasRelevantTransition) { + const lastCheckAt = lastReviewBacklogCheckAt.get(session.id) ?? 0; + if (Date.now() - lastCheckAt < REVIEW_BACKLOG_THROTTLE_MS) { + return; + } + } + lastReviewBacklogCheckAt.set(session.id, Date.now()); + const [pendingResult, automatedResult] = await Promise.allSettled([ scm.getPendingComments(session.pr), scm.getAutomatedComments(session.pr), @@ -941,13 +973,22 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return; } - // Fetch individual CI checks for failure details + // Fetch individual CI checks for failure details. + // Use batch enrichment data when available to avoid an extra REST call; + // fall back to getCIChecks() when the batch didn't run this cycle. + const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`; + const cachedEnrichment = prEnrichmentCache.get(prKey); + let checks: CICheck[]; - try { - checks = await scm.getCIChecks(session.pr); - } catch { - // Failed to fetch checks — skip this cycle - return; + if (cachedEnrichment?.ciChecks !== undefined) { + checks = cachedEnrichment.ciChecks; + } else { + try { + checks = await scm.getCIChecks(session.pr); + } catch { + // Failed to fetch checks — skip this cycle + return; + } } const failedChecks = checks.filter( @@ -1058,14 +1099,19 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return; } - // Check for conflicts using cached enrichment data or fallback to individual call + // Check for conflicts using cached enrichment data or fallback to individual call. + // When batch enrichment ran (cachedData is present), use its hasConflicts value + // to avoid 3 redundant REST calls from getMergeability() — the batch already + // fetched the mergeable/mergeStateStatus fields via GraphQL. const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`; const cachedData = prEnrichmentCache.get(prKey); let hasConflicts: boolean; - if (cachedData && cachedData.hasConflicts !== undefined) { - hasConflicts = cachedData.hasConflicts; + if (cachedData) { + // Batch ran — trust its data (undefined means CONFLICTING wasn't set → no conflicts) + hasConflicts = cachedData.hasConflicts ?? false; } else { + // Batch didn't run this cycle — fall back to individual API call try { const mergeReadiness = await scm.getMergeability(session.pr); hasConflicts = !mergeReadiness.noConflicts; @@ -1256,8 +1302,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan // Poll all sessions concurrently await Promise.allSettled(sessionsToCheck.map((s) => checkSession(s))); - // Prune stale entries from states and reactionTrackers for sessions - // that no longer appear in the session list (e.g., after kill/cleanup) + // Prune stale entries from states, reactionTrackers, and lastReviewBacklogCheckAt + // for sessions that no longer appear in the session list (e.g., after kill/cleanup) const currentSessionIds = new Set(sessions.map((s) => s.id)); for (const trackedId of states.keys()) { if (!currentSessionIds.has(trackedId)) { @@ -1270,6 +1316,11 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan reactionTrackers.delete(trackerKey); } } + for (const sessionId of lastReviewBacklogCheckAt.keys()) { + if (!currentSessionIds.has(sessionId)) { + lastReviewBacklogCheckAt.delete(sessionId); + } + } // Check if all sessions are complete (trigger reaction only once) const activeSessions = sessions.filter((s) => !TERMINAL_STATUSES.has(s.status)); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 1d6d0eee0..0416bd6b3 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -816,6 +816,8 @@ export interface PREnrichmentData { isBehind?: boolean; /** List of blockers preventing merge */ blockers?: string[]; + /** Individual CI check results (populated from batch enrichment when available) */ + ciChecks?: CICheck[]; } /** diff --git a/packages/plugins/scm-github/src/graphql-batch.ts b/packages/plugins/scm-github/src/graphql-batch.ts index 783d207a0..f877e3d58 100644 --- a/packages/plugins/scm-github/src/graphql-batch.ts +++ b/packages/plugins/scm-github/src/graphql-batch.ts @@ -9,6 +9,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import type { BatchObserver, + CICheck, CIStatus, PREnrichmentData, PRInfo, @@ -466,6 +467,24 @@ const PR_FIELDS = ` commit { statusCheckRollup { state + contexts(first: 20) { + nodes { + ... on CheckRun { + name + status + conclusion + detailsUrl + } + ... on StatusContext { + context + state + targetUrl + } + } + pageInfo { + hasNextPage + } + } } } } @@ -574,12 +593,108 @@ async function executeBatchQuery( return (result.data ?? {}) as Record; } +/** + * Parse individual CI check contexts from statusCheckRollup.contexts.nodes. + * Handles both CheckRun (GitHub Actions) and StatusContext (legacy status checks). + */ +function parseCheckContexts(contexts: unknown): CICheck[] { + if (!contexts || typeof contexts !== "object") return []; + + const nodes = (contexts as Record)["nodes"]; + if (!Array.isArray(nodes)) return []; + + const checks: CICheck[] = []; + for (const node of nodes) { + if (!node || typeof node !== "object") continue; + const n = node as Record; + + // CheckRun node (GitHub Actions) + if (typeof n["name"] === "string" && typeof n["status"] === "string") { + const rawStatus = (n["status"] as string).toUpperCase(); + // Uppercase conclusion to match REST getCIChecks/getCIChecksFromStatusRollup format + // so fingerprints are consistent regardless of which data source is used. + const rawConclusion = + typeof n["conclusion"] === "string" ? (n["conclusion"] as string).toUpperCase() : null; + + let status: CICheck["status"]; + if (rawStatus === "COMPLETED") { + if (rawConclusion === "SUCCESS") { + status = "passed"; + } else if ( + rawConclusion === "SKIPPED" || + rawConclusion === "NEUTRAL" || + rawConclusion === "STALE" || + rawConclusion === "NOT_REQUIRED" || + rawConclusion === "NONE" + ) { + // Mirror mapRawCheckStateToStatus() in the REST path: all non-failure + // terminal conclusions that are not SUCCESS map to "skipped". + status = "skipped"; + } else if ( + rawConclusion === "FAILURE" || + rawConclusion === "TIMED_OUT" || + rawConclusion === "CANCELLED" || + rawConclusion === "ACTION_REQUIRED" || + rawConclusion === "ERROR" + ) { + // Explicit failure conclusions — mirrors the failure list in mapRawCheckStateToStatus() + status = "failed"; + } else { + // STARTUP_FAILURE and any other unrecognized conclusion → "skipped", + // matching mapRawCheckStateToStatus()'s default return "skipped" in the REST path. + status = "skipped"; + } + } else if (rawStatus === "IN_PROGRESS") { + // Only IN_PROGRESS maps to "running" — matches mapRawCheckStateToStatus() in REST path + status = "running"; + } else { + // QUEUED, WAITING, and any other non-COMPLETED status → "pending" + // (REST path maps QUEUED/WAITING to "pending", not "running") + status = "pending"; + } + + checks.push({ + name: n["name"] as string, + status, + // Store the uppercased conclusion to match REST format + conclusion: rawConclusion ?? undefined, + url: typeof n["detailsUrl"] === "string" ? (n["detailsUrl"] as string) : undefined, + }); + continue; + } + + // StatusContext node (legacy commit statuses) + if (typeof n["context"] === "string" && typeof n["state"] === "string") { + const rawState = (n["state"] as string).toUpperCase(); + let status: CICheck["status"]; + if (rawState === "SUCCESS") { + status = "passed"; + } else if (rawState === "FAILURE" || rawState === "ERROR") { + status = "failed"; + } else { + status = "pending"; + } + + // Set conclusion to match the REST getCIChecksFromStatusRollup format + // (which sets conclusion = rawState.toUpperCase()) so fingerprints are + // consistent regardless of which data source is used. + checks.push({ + name: n["context"] as string, + status, + conclusion: rawState, + url: typeof n["targetUrl"] === "string" ? (n["targetUrl"] as string) : undefined, + }); + } + } + + return checks; +} + /** * Parse raw CI state from status check rollup. * - * Uses only the top-level aggregate state, which is much cheaper - * than fetching individual check contexts. The top-level state - * provides the same semantic information (passing/failing/pending). + * Uses only the top-level aggregate state to determine overall CI status. + * Individual check details are parsed separately via parseCheckContexts(). */ function parseCIState( statusCheckRollup: unknown, @@ -679,13 +794,30 @@ function extractPREnrichment( // Extract review decision const reviewDecision = parseReviewDecision(pr["reviewDecision"]); - // Extract CI status from commits + // Extract CI status and individual checks from commits const commits = pr["commits"] as - | { nodes?: Array<{ commit?: { statusCheckRollup?: unknown } }> } + | { nodes?: Array<{ commit?: { statusCheckRollup?: Record } }> } | undefined; - const ciStatus = commits?.nodes?.[0]?.commit?.statusCheckRollup - ? parseCIState(commits.nodes[0].commit.statusCheckRollup) - : "none"; + const statusCheckRollup = commits?.nodes?.[0]?.commit?.statusCheckRollup; + const ciStatus = statusCheckRollup ? parseCIState(statusCheckRollup) : "none"; + + // Only include ciChecks when the list is complete (no truncation). + // contexts(first: 20) silently truncates PRs with >20 checks — when truncated, + // the failing check may be missing, so we set ciChecks to undefined to force + // the getCIChecks() REST fallback in maybeDispatchCIFailureDetails. + const contextsField = statusCheckRollup?.["contexts"] as + | Record + | undefined; + const pageInfo = contextsField?.["pageInfo"]; + const contextsHasNextPage = + pageInfo !== null && + pageInfo !== undefined && + typeof pageInfo === "object" && + (pageInfo as Record)["hasNextPage"] === true; + const ciChecks = + contextsField && !contextsHasNextPage + ? parseCheckContexts(contextsField) + : undefined; // Build blockers list const blockers: string[] = []; @@ -722,6 +854,7 @@ function extractPREnrichment( hasConflicts, isBehind, blockers, + ...(ciChecks !== undefined ? { ciChecks } : {}), }; return { data, headSha }; diff --git a/packages/plugins/scm-github/test/graphql-batch.test.ts b/packages/plugins/scm-github/test/graphql-batch.test.ts index a25417087..cdab3eea2 100644 --- a/packages/plugins/scm-github/test/graphql-batch.test.ts +++ b/packages/plugins/scm-github/test/graphql-batch.test.ts @@ -1172,3 +1172,576 @@ describe("shouldRefreshPREnrichment - ETag Guard Strategy", () => { }); }); }); + +describe("extractPREnrichment ciChecks", () => { + it("parses CheckRun contexts into ciChecks", () => { + const pullRequest = { + title: "Fix CI", + state: "OPEN", + additions: 10, + deletions: 5, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { + name: "lint", + status: "COMPLETED", + conclusion: "FAILURE", + detailsUrl: "https://github.com/org/repo/actions/runs/123", + }, + { + name: "test", + status: "COMPLETED", + conclusion: "SUCCESS", + detailsUrl: "https://github.com/org/repo/actions/runs/124", + }, + { + name: "typecheck", + status: "IN_PROGRESS", + conclusion: null, + detailsUrl: "https://github.com/org/repo/actions/runs/125", + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const ciChecks = extracted?.data.ciChecks; + + expect(ciChecks).toBeDefined(); + expect(ciChecks).toHaveLength(3); + + const lint = ciChecks?.find((c) => c.name === "lint"); + expect(lint?.status).toBe("failed"); + expect(lint?.conclusion).toBe("FAILURE"); + expect(lint?.url).toBe("https://github.com/org/repo/actions/runs/123"); + + const test = ciChecks?.find((c) => c.name === "test"); + expect(test?.status).toBe("passed"); + expect(test?.conclusion).toBe("SUCCESS"); + + const typecheck = ciChecks?.find((c) => c.name === "typecheck"); + expect(typecheck?.status).toBe("running"); + }); + + it("maps QUEUED and WAITING to pending, not running (matches REST mapRawCheckStateToStatus)", () => { + const pullRequest = { + title: "Queued checks", + state: "OPEN", + additions: 1, + deletions: 0, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "PENDING", + contexts: { + nodes: [ + { name: "queued-check", status: "QUEUED", conclusion: null, detailsUrl: null }, + { name: "waiting-check", status: "WAITING", conclusion: null, detailsUrl: null }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const ciChecks = extracted?.data.ciChecks; + expect(ciChecks?.find((c) => c.name === "queued-check")?.status).toBe("pending"); + expect(ciChecks?.find((c) => c.name === "waiting-check")?.status).toBe("pending"); + }); + + it("parses StatusContext nodes into ciChecks", () => { + const pullRequest = { + title: "Legacy CI", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { + context: "ci/build", + state: "FAILURE", + targetUrl: "https://ci.example.com/build/1", + }, + { + context: "ci/test", + state: "SUCCESS", + targetUrl: "https://ci.example.com/test/1", + }, + { + context: "ci/deploy", + state: "PENDING", + targetUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const ciChecks = extracted?.data.ciChecks; + + expect(ciChecks).toBeDefined(); + expect(ciChecks).toHaveLength(3); + + const build = ciChecks?.find((c) => c.name === "ci/build"); + expect(build?.status).toBe("failed"); + expect(build?.conclusion).toBe("FAILURE"); // matches REST getCIChecksFromStatusRollup format + expect(build?.url).toBe("https://ci.example.com/build/1"); + + const test = ciChecks?.find((c) => c.name === "ci/test"); + expect(test?.status).toBe("passed"); + expect(test?.conclusion).toBe("SUCCESS"); + + const deploy = ciChecks?.find((c) => c.name === "ci/deploy"); + expect(deploy?.status).toBe("pending"); + expect(deploy?.conclusion).toBe("PENDING"); + expect(deploy?.url).toBeUndefined(); + }); + + it("returns empty array when contexts nodes is empty", () => { + const pullRequest = { + title: "Clean PR", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { nodes: [] }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + expect(extracted?.data.ciChecks).toEqual([]); + }); + + it("returns undefined ciChecks when statusCheckRollup has no contexts field", () => { + const pullRequest = { + title: "No contexts", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + // No contexts field — older API response + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + expect(extracted?.data.ciChecks).toBeUndefined(); + }); + + it("maps COMPLETED+SKIPPED conclusion to skipped status", () => { + const pullRequest = { + title: "Skipped check", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { + nodes: [ + { + name: "optional-check", + status: "COMPLETED", + conclusion: "SKIPPED", + detailsUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const check = extracted?.data.ciChecks?.[0]; + expect(check?.status).toBe("skipped"); + expect(check?.conclusion).toBe("SKIPPED"); // uppercased to match REST format + }); + + it("maps COMPLETED+NEUTRAL conclusion to skipped (matches REST mapRawCheckStateToStatus)", () => { + const pullRequest = { + title: "Neutral check", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { + nodes: [ + { + name: "optional-check", + status: "COMPLETED", + conclusion: "NEUTRAL", + detailsUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const check = extracted?.data.ciChecks?.[0]; + // NEUTRAL maps to "skipped" in REST mapRawCheckStateToStatus — must match + expect(check?.status).toBe("skipped"); + expect(check?.conclusion).toBe("NEUTRAL"); + }); + + it("uppercases CheckRun conclusion to match REST getCIChecks format", () => { + const pullRequest = { + title: "Mixed case conclusion", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { + name: "lint", + status: "COMPLETED", + conclusion: "failure", // lowercase from API + detailsUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const check = extracted?.data.ciChecks?.[0]; + expect(check?.status).toBe("failed"); + expect(check?.conclusion).toBe("FAILURE"); // must be uppercased + }); + + it("maps STALE/NOT_REQUIRED/NONE conclusions to skipped (matches REST mapRawCheckStateToStatus)", () => { + const makeContextsWithConclusion = (conclusion: string) => ({ + title: "Check", + state: "OPEN", + additions: 1, + deletions: 0, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { + nodes: [{ name: "check", status: "COMPLETED", conclusion, detailsUrl: null }], + }, + }, + }, + }, + ], + }, + }); + + for (const conclusion of ["STALE", "NOT_REQUIRED", "NONE"]) { + const extracted = extractPREnrichment(makeContextsWithConclusion(conclusion)); + const check = extracted?.data.ciChecks?.[0]; + expect(check?.status, `${conclusion} should map to skipped`).toBe("skipped"); + } + }); + + it("returns undefined ciChecks when contexts list is truncated (hasNextPage=true)", () => { + const pullRequest = { + title: "Many CI checks", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { name: "check-1", status: "COMPLETED", conclusion: "FAILURE", detailsUrl: null }, + // ... 19 more checks truncated + ], + pageInfo: { hasNextPage: true }, // list was truncated! + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + // ciChecks should be undefined when truncated — forces getCIChecks() REST fallback + expect(extracted?.data.ciChecks).toBeUndefined(); + }); + + it("returns ciChecks when contexts list is complete (hasNextPage=false)", () => { + const pullRequest = { + title: "Few CI checks", + state: "OPEN", + additions: 5, + deletions: 2, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { name: "lint", status: "COMPLETED", conclusion: "FAILURE", detailsUrl: "https://ci.example.com/lint" }, + ], + pageInfo: { hasNextPage: false }, // complete list + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + expect(extracted?.data.ciChecks).toBeDefined(); + expect(extracted?.data.ciChecks).toHaveLength(1); + expect(extracted?.data.ciChecks?.[0]?.name).toBe("lint"); + }); + + it("maps COMPLETED with null conclusion to skipped (matches REST mapRawCheckStateToStatus empty-string branch)", () => { + // REST path: mapRawCheckStateToStatus(undefined) → state="" → "skipped" + // GraphQL path must match: COMPLETED + null conclusion → "skipped", not "passed" + const pullRequest = { + title: "Null conclusion check", + state: "OPEN", + additions: 1, + deletions: 0, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { + nodes: [ + { + name: "some-check", + status: "COMPLETED", + conclusion: null, // null conclusion — shouldn't map to "passed" + detailsUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const check = extracted?.data.ciChecks?.[0]; + expect(check?.status).toBe("skipped"); + }); + + it("maps COMPLETED+STARTUP_FAILURE to skipped (matches REST mapRawCheckStateToStatus default fallback)", () => { + const pullRequest = { + title: "Startup failure check", + state: "OPEN", + additions: 1, + deletions: 0, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "FAILURE", + contexts: { + nodes: [ + { + name: "infra-check", + status: "COMPLETED", + conclusion: "STARTUP_FAILURE", + detailsUrl: null, + }, + ], + }, + }, + }, + }, + ], + }, + }; + + const extracted = extractPREnrichment(pullRequest); + const check = extracted?.data.ciChecks?.[0]; + // STARTUP_FAILURE is not in the explicit failure list → falls through to "skipped" + // matching mapRawCheckStateToStatus()'s default return "skipped" + expect(check?.status).toBe("skipped"); + expect(check?.conclusion).toBe("STARTUP_FAILURE"); + }); + + it("handles null pageInfo safely without TypeError (typeof null === 'object' quirk)", () => { + const pullRequest = { + title: "Null pageInfo", + state: "OPEN", + additions: 1, + deletions: 0, + isDraft: false, + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: "NONE", + reviews: { nodes: [] }, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + state: "SUCCESS", + contexts: { + nodes: [ + { name: "lint", status: "COMPLETED", conclusion: "SUCCESS", detailsUrl: null }, + ], + pageInfo: null, // null pageInfo — older API responses may omit this + }, + }, + }, + }, + ], + }, + }; + + // Must not throw TypeError: Cannot read properties of null + expect(() => extractPREnrichment(pullRequest)).not.toThrow(); + const extracted = extractPREnrichment(pullRequest); + // null pageInfo is treated as "not truncated" → ciChecks should be defined + expect(extracted?.data.ciChecks).toBeDefined(); + expect(extracted?.data.ciChecks).toHaveLength(1); + }); +});