From 784bbc5d4ebef13c2b1c6743e05ce14c2dd25892 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 21:33:13 -0400 Subject: [PATCH 01/10] feat(poll): classify running/waiting/pending checks and narrow hot-poll --- .../components/dashboard/DashboardPage.tsx | 19 +- src/app/components/shared/StatusDot.tsx | 12 +- src/app/services/api.ts | 99 ++-- src/app/services/poll.ts | 105 ++++- src/shared/types.ts | 18 +- tests/components/DashboardPage.test.tsx | 38 +- tests/lib/dependency-detection.test.ts | 20 +- tests/lib/format.test.ts | 4 +- tests/services/api.test.ts | 184 +++++++- tests/services/events-poll.test.ts | 12 +- tests/services/hot-poll.test.ts | 425 ++++++++++++++++-- 11 files changed, 811 insertions(+), 125 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 8d4a77d3..d7634de6 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -580,7 +580,7 @@ async function pollFetch(force = false): Promise { recentlyMarkedReadyPRs.size > 0 || recentlyApprovedPRs.size > 0 ? resolvedPullRequests.map((pr) => (recentlyMarkedReadyPRs.has(pr.id) || recentlyApprovedPRs.has(pr.id)) && pr.nodeId - ? { ...pr, enriched: true, checkStatus: "pending" as const } + ? { ...pr, enriched: true, checkStatus: "running" as const, hotPollPinned: true } : pr ) : resolvedPullRequests; @@ -702,6 +702,8 @@ function handleTargetedData(data: DashboardData, affectedRepos: string[]): void applyRecentApprovalGuard(pr, currentLogin); } + const targetedRuns = config.enableActions ? data.workflowRuns : []; + withScrollLock(() => { setDashboardData(produce((state) => { // ID-based merge: replace targeted items, keep unaffected + tracked-user-only items @@ -721,7 +723,6 @@ function handleTargetedData(data: DashboardData, affectedRepos: string[]): void ), ...data.pullRequests, ]; - const targetedRuns = config.enableActions ? data.workflowRuns : []; const newRunIds = new Set(targetedRuns.map(r => r.id)); state.workflowRuns = config.enableActions ? [ ...state.workflowRuns.filter(r => @@ -757,7 +758,15 @@ function handleTargetedData(data: DashboardData, affectedRepos: string[]): void const newItems = detectNewItems(data); dispatchNotifications(newItems, config); - seedHotSetsFromTargeted(data); + const hotTargetedPullRequests = + recentlyMarkedReadyPRs.size > 0 || recentlyApprovedPRs.size > 0 + ? data.pullRequests.map((pr) => + (recentlyMarkedReadyPRs.has(pr.id) || recentlyApprovedPRs.has(pr.id)) && pr.nodeId + ? { ...pr, enriched: true, checkStatus: "running" as const, hotPollPinned: true } + : pr + ) + : data.pullRequests; + seedHotSetsFromTargeted({ ...data, pullRequests: hotTargetedPullRequests, workflowRuns: targetedRuns }); persistDashboardSnapshot(); } @@ -1825,7 +1834,7 @@ export default function DashboardPage() { // even when checkStatus was already "success" prior to approval. seedHotSetsFromTargeted({ issues: [], - pullRequests: [{ ...pr, nodeId, headSha: approvedHeadSha, enriched: true, checkStatus: "pending" }], + pullRequests: [{ ...pr, nodeId, headSha: approvedHeadSha, enriched: true, checkStatus: "running", hotPollPinned: true }], workflowRuns: [], errors: [], }); @@ -1894,7 +1903,7 @@ export default function DashboardPage() { seedHotSetsFromTargeted({ issues: [], - pullRequests: [{ ...pr, nodeId, draft: false, enriched: true, checkStatus: "pending" }], + pullRequests: [{ ...pr, nodeId, draft: false, enriched: true, checkStatus: "running", hotPollPinned: true }], workflowRuns: [], errors: [], }); diff --git a/src/app/components/shared/StatusDot.tsx b/src/app/components/shared/StatusDot.tsx index b3b68734..0412f2f7 100644 --- a/src/app/components/shared/StatusDot.tsx +++ b/src/app/components/shared/StatusDot.tsx @@ -2,7 +2,7 @@ import { Show } from "solid-js"; import { Tooltip } from "./Tooltip"; export interface StatusDotProps { - status: "success" | "pending" | "failure" | "error" | "conflict" | null; + status: "success" | "running" | "waiting" | "pending" | "failure" | "error" | "conflict" | null; href?: string; } @@ -12,6 +12,16 @@ const STATUS_CONFIG = { label: "All checks passed", pulse: false, }, + running: { + bg: "bg-warning", + label: "Checks in progress", + pulse: true, + }, + waiting: { + bg: "bg-amber-500", + label: "Checks pending human action", + pulse: false, + }, pending: { bg: "bg-warning", label: "Checks in progress", diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 32bd7225..fbf5d94e 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -204,6 +204,20 @@ interface GraphQLIssueSearchResponse { rateLimit?: GraphQLRateLimit; } +export interface GraphQLRollupStateCount { + state: string; + count: number; +} + +export interface GraphQLStatusCheckRollup { + state: string; + contexts?: { + checkRunCountsByState?: GraphQLRollupStateCount[] | null; + } | null; +} + +const STATUS_CHECK_ROLLUP_FIELDS = `state contexts { checkRunCountsByState { state count } }`; + interface ForkCandidate { databaseId: number; headOwner: string; @@ -212,7 +226,7 @@ interface ForkCandidate { } interface ForkRepoResult { - object: { statusCheckRollup: { state: string } | null } | null; + object: { statusCheckRollup: GraphQLStatusCheckRollup | null } | null; } interface ForkQueryResponse { @@ -417,7 +431,7 @@ const HEAVY_PR_BACKFILL_QUERY = ` nodes(ids: $ids) { ... on PullRequest {${BASE_PR_BACKFILL_FIELDS} mergeStateStatus - statusCheckRollup { state } + statusCheckRollup { ${STATUS_CHECK_ROLLUP_FIELDS} } } } rateLimit { cost limit remaining resetAt } @@ -450,9 +464,9 @@ const HOT_PR_STATUS_QUERY = ` autoMergeRequest { mergeMethod } viewerLatestReview { state } viewerLatestReviewRequest { id } - statusCheckRollup { state } + statusCheckRollup { ${STATUS_CHECK_ROLLUP_FIELDS} } isCrossRepository - headRef { target { ... on Commit { statusCheckRollup { state } } } } + headRef { target { ... on Commit { statusCheckRollup { ${STATUS_CHECK_ROLLUP_FIELDS} } } } } } } rateLimit { cost limit remaining resetAt } @@ -470,9 +484,9 @@ interface HotPRStatusNode { autoMergeRequest?: { mergeMethod: string } | null; viewerLatestReview?: { state: string } | null; viewerLatestReviewRequest?: { id: string } | null; - statusCheckRollup?: { state: string } | null; + statusCheckRollup?: GraphQLStatusCheckRollup | null; isCrossRepository?: boolean; - headRef?: { target?: { statusCheckRollup?: { state: string } | null } | null } | null; + headRef?: { target?: { statusCheckRollup?: GraphQLStatusCheckRollup | null } | null } | null; } interface HotPRStatusResponse { @@ -518,7 +532,7 @@ interface GraphQLHeavyPRNode { autoMergeRequest?: { mergeMethod: string } | null; viewerLatestReview?: { state: string } | null; viewerLatestReviewRequest?: { id: string } | null; - statusCheckRollup?: { state: string } | null; + statusCheckRollup?: GraphQLStatusCheckRollup | null; } interface LightCombinedSearchResponse { @@ -718,7 +732,7 @@ async function runForkPRFallback( `fork${i}: repository(owner: $owner${i}, name: $repo${i}) { object(expression: $sha${i}) { ... on Commit { - statusCheckRollup { state } + statusCheckRollup { ${STATUS_CHECK_ROLLUP_FIELDS} } } } }` @@ -735,10 +749,10 @@ async function runForkPRFallback( for (let i = 0; i < forkChunk.length; i++) { const data = forkResponse[`fork${i}`] as ForkRepoResult | null | undefined; - const state = data?.object?.statusCheckRollup?.state ?? null; + const rollup = data?.object?.statusCheckRollup ?? null; const pr = prMap.get(forkChunk[i].databaseId); if (pr) { - pr.checkStatus = mapCheckStatus(state); + pr.checkStatus = mapCheckStatus(rollup); } } } catch (err) { @@ -753,8 +767,8 @@ async function runForkPRFallback( const pr = prMap.get(forkChunk[i].databaseId); if (!pr) continue; if (data) { - const state = data.object?.statusCheckRollup?.state ?? null; - pr.checkStatus = mapCheckStatus(state); + const rollup = data.object?.statusCheckRollup ?? null; + pr.checkStatus = mapCheckStatus(rollup); } else { pr.checkStatusStale = true; } @@ -1134,8 +1148,8 @@ export interface PREnrichmentData { viewerLatestReviewState: ViewerReviewState | null; } -function extractRollupState(node: { statusCheckRollup?: { state: string } | null }): string | null { - return node.statusCheckRollup?.state ?? null; +function extractRollup(node: { statusCheckRollup?: GraphQLStatusCheckRollup | null }): GraphQLStatusCheckRollup | null { + return node.statusCheckRollup ?? null; } function mapHeavyPRNode(node: GraphQLHeavyPRNode, isLightFallback: boolean): PREnrichmentData { @@ -1151,7 +1165,7 @@ function mapHeavyPRNode(node: GraphQLHeavyPRNode, isLightFallback: boolean): PRE ? null : applyMergeStateOverride( node.mergeStateStatus ?? "", - mapCheckStatus(extractRollupState(node)), + mapCheckStatus(extractRollup(node)), ); let headRepository: PREnrichmentData["headRepository"] = null; @@ -1922,7 +1936,7 @@ export async function fetchIssuesAndPullRequests( cached.userLogin === normalizedLogin && cached.updatedAt === pr.updatedAt && !cached.enrichment.checkStatusStale && - cached.enrichment.checkStatus !== "pending" && + !isNonTerminalCheckStatus(cached.enrichment.checkStatus) && Date.now() - cached.fetchedAt < _enrichmentCacheTtlMs ) { return cached.enrichment; @@ -2092,7 +2106,7 @@ export async function fetchIssuesAndPullRequests( for (const [prId, rawEnrichment] of freshEnrichments) { const finalPr = enrichedPRMap.get(prId); const nodeId = nodeIdMap.get(prId); - if (finalPr && nodeId && !finalPr.checkStatusStale && finalPr.checkStatus !== "pending") { + if (finalPr && nodeId && !finalPr.checkStatusStale && !isNonTerminalCheckStatus(finalPr.checkStatus)) { _prEnrichmentCache.set(nodeId, { userLogin: normalizedLogin, updatedAt: finalPr.updatedAt, @@ -2129,19 +2143,50 @@ export async function fetchIssuesAndPullRequests( } /** - * Maps a GraphQL statusCheckRollup state string to the app's CheckStatus type. + * True when a PR's checkStatus is non-terminal ("running", "waiting", or "pending"), + * meaning CI has not reached a settled pass/fail/conflict state. + */ +export function isNonTerminalCheckStatus(status: CheckStatus["status"]): boolean { + return status === "running" || status === "waiting" || status === "pending"; +} + +/** + * Maps a GraphQL statusCheckRollup object (or state string) to the app's CheckStatus["status"]. + * Non-terminal rollups (PENDING / EXPECTED / QUEUED) inspect contexts.checkRunCountsByState: + * - IN_PROGRESS > 0 -> "running" + * - WAITING > 0 or ACTION_REQUIRED > 0 -> "waiting" + * - otherwise (e.g. QUEUED > 0, pending StatusContext, or omitted contexts) -> "pending" */ -function mapCheckStatus(state: string | null | undefined): CheckStatus["status"] { +function mapCheckStatus(rollup: GraphQLStatusCheckRollup | string | null | undefined): CheckStatus["status"] { + const state = typeof rollup === "string" ? rollup : rollup?.state ?? null; if (state === "FAILURE" || state === "ERROR" || state === "ACTION_REQUIRED") return "failure"; - if (state === "PENDING" || state === "EXPECTED" || state === "QUEUED") return "pending"; if (state === "SUCCESS") return "success"; + if (state === "PENDING" || state === "EXPECTED" || state === "QUEUED") { + const checkRunCounts = + typeof rollup === "object" && rollup !== null + ? rollup.contexts?.checkRunCountsByState + : undefined; + if (Array.isArray(checkRunCounts)) { + if (checkRunCounts.some((c) => c.state === "IN_PROGRESS" && c.count > 0)) return "running"; + if ( + checkRunCounts.some( + (c) => (c.state === "WAITING" || c.state === "ACTION_REQUIRED") && c.count > 0, + ) + ) { + return "waiting"; + } + return "pending"; + } + return "pending"; + } 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). + * DIRTY/BEHIND → conflict. UNSTABLE → failure unless checkStatus is non-terminal + * ("running", "waiting", or "pending"), since a non-terminal rollup means checks + * are still active or awaiting action/conditions rather than conclusively failed. * All other values (CLEAN, BLOCKED, HAS_HOOKS, UNKNOWN) pass through unchanged. */ function applyMergeStateOverride( @@ -2149,7 +2194,7 @@ function applyMergeStateOverride( checkStatus: CheckStatus["status"], ): CheckStatus["status"] { if (mergeStateStatus === "DIRTY" || mergeStateStatus === "BEHIND") return "conflict"; - if (mergeStateStatus === "UNSTABLE" && checkStatus !== "pending") return "failure"; + if (mergeStateStatus === "UNSTABLE" && !isNonTerminalCheckStatus(checkStatus)) return "failure"; return checkStatus; } @@ -2462,10 +2507,10 @@ export async function fetchHotPRStatus( // Fork PRs whose CI runs only in the fork report no PR-level rollup; read the // fork head commit's rollup instead (the same source runForkPRFallback uses). - const rollupState = - extractRollupState(node) ?? - (node.isCrossRepository ? (node.headRef?.target?.statusCheckRollup?.state ?? null) : null); - const checkStatus = applyMergeStateOverride(node.mergeStateStatus, mapCheckStatus(rollupState)); + const rollup = + extractRollup(node) ?? + (node.isCrossRepository ? (node.headRef?.target?.statusCheckRollup ?? null) : null); + const checkStatus = applyMergeStateOverride(node.mergeStateStatus, mapCheckStatus(rollup)); results.set(node.databaseId, { state: node.state, diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index 38ad962c..9343b3b6 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -57,11 +57,14 @@ export interface PollCoordinator { // ── Hot poll state ──────────────────────────────────────────────────────────── -/** PRs with pending/null check status: maps GraphQL node ID → databaseId */ +/** PRs with running/stale/pinned check status: maps GraphQL node ID → databaseId */ const _hotPRs = new Map(); /** Inverse index for O(1) eviction: maps databaseId → nodeId */ const _hotPRsByDbId = new Map(); const MAX_HOT_PRS = 200; +const HOT_PR_MUTATION_PIN_TTL_MS = 60_000; +/** Post-mutation pin deadlines: maps databaseId → expiry timestamp (ms) */ +const _hotPRPinnedUntil = new Map(); /** In-progress workflow runs: maps run ID → repo descriptor */ const _hotRuns = new Map(); @@ -87,6 +90,7 @@ export function clearHotSets(): void { _hotPRs.clear(); _hotPRsByDbId.clear(); _hotRuns.clear(); + _hotPRPinnedUntil.clear(); } export const WAKE_GRACE_MS = 2_000; @@ -137,9 +141,7 @@ export function processBackfillErrors( } export function resetPollState(): void { - _hotPRs.clear(); - _hotPRsByDbId.clear(); - _hotRuns.clear(); + clearHotSets(); _hotPollGeneration = 0; _consecutiveBackfillFailures = 0; resetPREnrichmentCache(); @@ -588,7 +590,9 @@ export function createPollCoordinator( /** * Rebuilds hot item sets from fresh full refresh data. Called after each full * poll cycle completes. Clears and replaces both sets (full replacement, not - * incremental). Increments the generation counter so stale hot poll results + * incremental). Enrolls PRs whose checkStatus is actively "running", whose + * checkStatusStale flag is set, or whose post-mutation hotPollPinned window + * is active. Increments the generation counter so stale hot poll results * from the previous cycle can be detected and discarded. */ export function rebuildHotSets(data: DashboardData): void { @@ -597,8 +601,27 @@ export function rebuildHotSets(data: DashboardData): void { _hotPRsByDbId.clear(); _hotRuns.clear(); + const now = Date.now(); + for (const [id, untilMs] of _hotPRPinnedUntil) { + if (untilMs <= now) _hotPRPinnedUntil.delete(id); + } + for (const pr of data.pullRequests) { - if (pr.enriched && (pr.checkStatus === "pending" || pr.checkStatusStale === true) && pr.nodeId) { + if (pr.hotPollPinned) { + if ((_hotPRPinnedUntil.get(pr.id) ?? 0) <= now) { + _hotPRPinnedUntil.set(pr.id, now + HOT_PR_MUTATION_PIN_TTL_MS); + } + } else if (_hotPRPinnedUntil.has(pr.id)) { + _hotPRPinnedUntil.delete(pr.id); + } + + if ( + pr.enriched && + pr.nodeId && + (pr.checkStatus === "running" || + pr.checkStatusStale === true || + (_hotPRPinnedUntil.get(pr.id) ?? 0) > now) + ) { if (_hotPRs.size >= MAX_HOT_PRS) { console.warn(`[hot-poll] PR cap reached (${MAX_HOT_PRS}), skipping remaining`); break; @@ -623,10 +646,11 @@ export function rebuildHotSets(data: DashboardData): void { } /** - * Fetches updated status for all hot items (pending-check PRs + in-progress runs). - * Evicts items from the hot sets when they settle (PR closed/merged/resolved, - * run completed). Returns captured generation alongside results so callers can - * detect staleness. + * Fetches updated status for all hot items (running/stale/pinned PRs + in-progress/queued runs). + * Evicts items from the hot sets when they leave active execution (PR closed/merged or + * checkStatus !== "running" outside a post-mutation pin window; workflow run status no longer + * "in_progress" or "queued", e.g. "waiting" or "completed"). Returns captured generation + * alongside results so callers can detect staleness. * * @param priorConsecutiveFailures - Failures from prior cycles (before this one). * Below HOT_POLL_SENTRY_THRESHOLD, fetch errors are logged/surfaced to the user @@ -693,13 +717,16 @@ export async function fetchHotData(priorConsecutiveFailures = 0): Promise<{ // The freshly rebuilt sets are authoritative — evicting from them based on // stale fetch results would corrupt the new data. if (generation === _hotPollGeneration) { - // Evict settled PRs using inverse index for O(1) lookup + // Evict PRs that are no longer actively running (unless within a 60s post-mutation pin window) + const now = Date.now(); for (const [databaseId, upd] of prUpdates) { - if ( - upd.state === "CLOSED" || - upd.state === "MERGED" || - upd.checkStatus !== "pending" - ) { + const isTerminal = upd.state === "CLOSED" || upd.state === "MERGED"; + const pinnedUntil = _hotPRPinnedUntil.get(databaseId) ?? 0; + if (pinnedUntil > 0 && (isTerminal || now >= pinnedUntil)) { + _hotPRPinnedUntil.delete(databaseId); + } + const isPinned = !isTerminal && now < pinnedUntil; + if (isTerminal || (upd.checkStatus !== "running" && !isPinned)) { const nodeId = _hotPRsByDbId.get(databaseId); if (nodeId) { _hotPRs.delete(nodeId); @@ -708,9 +735,9 @@ export async function fetchHotData(priorConsecutiveFailures = 0): Promise<{ } } - // Evict completed runs + // Evict workflow runs that are no longer actively executing or queued (e.g. waiting or completed) for (const [runId, runUpdate] of runUpdates) { - if (runUpdate.status === "completed") { + if (runUpdate.status !== "in_progress" && runUpdate.status !== "queued") { _hotRuns.delete(runId); } } @@ -960,26 +987,58 @@ export async function fetchTargetedRepoData( // ── Hot set seeding from targeted refresh ──────────────────────────────────── +/** + * Updates hot item sets incrementally from a targeted refresh or post-mutation seed. + * Enrolls eligible running, stale, or post-mutation pinned PRs and queued/in_progress + * workflow runs, and evicts already-enrolled items in the targeted payload that have + * transitioned out of active execution (e.g. to "waiting", "pending", "success", or + * CLOSED/MERGED) without disturbing hot items in unaffected repositories. + */ export function seedHotSetsFromTargeted(data: DashboardData): void { + const now = Date.now(); + for (const [id, untilMs] of _hotPRPinnedUntil) { + if (untilMs <= now) _hotPRPinnedUntil.delete(id); + } + for (const pr of data.pullRequests) { - if (pr.enriched && (pr.checkStatus === "pending" || pr.checkStatusStale === true) && pr.nodeId) { - if (_hotPRs.size >= MAX_HOT_PRS) break; - if (!_hotPRs.has(pr.nodeId)) { + const isTerminal = pr.state === "CLOSED" || pr.state === "MERGED"; + if (!isTerminal && pr.hotPollPinned) { + if ((_hotPRPinnedUntil.get(pr.id) ?? 0) <= now) { + _hotPRPinnedUntil.set(pr.id, now + HOT_PR_MUTATION_PIN_TTL_MS); + } + } else { + _hotPRPinnedUntil.delete(pr.id); + } + + const isEligible = + !isTerminal && + Boolean(pr.enriched && pr.nodeId) && + (pr.checkStatus === "running" || + pr.checkStatusStale === true || + (_hotPRPinnedUntil.get(pr.id) ?? 0) > now); + + if (isEligible && pr.nodeId) { + if (!_hotPRs.has(pr.nodeId) && _hotPRs.size < MAX_HOT_PRS) { _hotPRs.set(pr.nodeId, pr.id); _hotPRsByDbId.set(pr.id, pr.nodeId); } + } else if (isTerminal || pr.enriched) { + const existingNodeId = _hotPRsByDbId.get(pr.id) ?? pr.nodeId; + if (existingNodeId) _hotPRs.delete(existingNodeId); + _hotPRsByDbId.delete(pr.id); } } for (const run of data.workflowRuns) { if (run.status === "queued" || run.status === "in_progress") { - if (_hotRuns.size >= MAX_HOT_RUNS) break; - if (!_hotRuns.has(run.id)) { + if (!_hotRuns.has(run.id) && _hotRuns.size < MAX_HOT_RUNS) { const parts = run.repoFullName.split("/"); if (parts.length === 2) { _hotRuns.set(run.id, { owner: parts[0], repo: parts[1] }); } } + } else { + _hotRuns.delete(run.id); } } } diff --git a/src/shared/types.ts b/src/shared/types.ts index 6e4ca1df..54f70e70 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -42,8 +42,18 @@ export interface Issue { nodeId?: string; } +/** + * CI check status for a pull request: + * - "success": all checks passed + * - "failure": one or more checks failed or errored + * - "running": non-terminal rollup with at least one check run IN_PROGRESS + * - "waiting": non-terminal rollup with zero IN_PROGRESS and at least one check run WAITING or ACTION_REQUIRED + * - "pending": non-terminal rollup with zero IN_PROGRESS and zero WAITING/ACTION_REQUIRED (e.g. QUEUED or pending StatusContext) + * - "conflict": PR is DIRTY or BEHIND base branch + * - null: no CI checks configured or not yet enriched + */ export interface CheckStatus { - status: "success" | "failure" | "pending" | "conflict" | null; + status: "success" | "failure" | "running" | "waiting" | "pending" | "conflict" | null; } export type AutoMergeMethod = "MERGE" | "SQUASH" | "REBASE"; @@ -97,6 +107,12 @@ export interface PullRequest { * and approve/mergeable gating treats CI as unknown. */ checkStatusStale?: boolean; + /** + * Transient flag set on post-mutation PR clones (Approve / Mark Ready) so + * hot-polling retains the PR during the 60s window before newly triggered + * workflows transition from QUEUED ("pending") into IN_PROGRESS ("running"). + */ + hotPollPinned?: boolean; /** GraphQL global node ID — used for hot-poll status updates */ nodeId?: string; /** PR body — fetched on demand for dependency update type detection */ diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index 2cd90dba..26621fb6 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -3670,7 +3670,7 @@ describe("DashboardPage — PR approval integration", () => { expect(errorsLib.dismissNotificationBySource).toHaveBeenCalledWith("approve-pr:42"); expect(pollService.seedHotSetsFromTargeted).toHaveBeenCalledWith({ issues: [], - pullRequests: [expect.objectContaining({ id: 42, enriched: true, checkStatus: "pending" })], + pullRequests: [expect.objectContaining({ id: 42, enriched: true, checkStatus: "running", hotPollPinned: true })], workflowRuns: [], errors: [], }); @@ -4009,7 +4009,7 @@ describe("DashboardPage — PR approval integration", () => { nodeId: "PR_kwDOtest0042", headSha: validSha, reviewDecision: "REVIEW_REQUIRED", - checkStatus: "pending", + checkStatus: "running", viewerReviewRequested: true, viewerLatestReviewState: null, }); @@ -4036,7 +4036,7 @@ describe("DashboardPage — PR approval integration", () => { capturedOnHotData!( new Map([[42, { state: "OPEN", - checkStatus: "pending", + checkStatus: "running", mergeStateStatus: "BLOCKED", reviewDecision: "REVIEW_REQUIRED", headSha: newSha, @@ -4108,7 +4108,7 @@ describe("DashboardPage — PR approval integration", () => { nodeId: "PR_kwDOtest0102", headSha: validSha, reviewDecision: "REVIEW_REQUIRED", - checkStatus: "pending", + checkStatus: "running", autoMergeMethod: null, viewerReviewRequested: true, viewerLatestReviewState: null, @@ -4533,7 +4533,8 @@ describe("DashboardPage — Mark PR ready integration", () => { nodeId: "PR_kwDOtest0077", draft: false, enriched: true, - checkStatus: "pending", + checkStatus: "running", + hotPollPinned: true, }), ], workflowRuns: [], @@ -4551,7 +4552,7 @@ describe("DashboardPage — Mark PR ready integration", () => { new Map([[77, { state: "OPEN", draft: true, - checkStatus: "pending", + checkStatus: "running", mergeStateStatus: "DRAFT", reviewDecision: null, }]]), @@ -4562,15 +4563,29 @@ describe("DashboardPage — Mark PR ready integration", () => { expect(screen.getByText("Review needed")).toBeTruthy(); // 2. Subsequent targeted poll reporting stale draft: true within 60s + vi.mocked(pollService.seedHotSetsFromTargeted).mockClear(); capturedOnTargetedData!( { issues: [], pullRequests: [makeStaleDraftPR()], workflowRuns: [], errors: [] }, ["owner/repo"] ); expect(screen.queryByTestId("draft-badge")).toBeNull(); expect(screen.getByText("Review needed")).toBeTruthy(); + expect(pollService.seedHotSetsFromTargeted).toHaveBeenCalledWith( + expect.objectContaining({ + pullRequests: [ + expect.objectContaining({ + id: 77, + nodeId: "PR_kwDOtest0077", + enriched: true, + checkStatus: "running", + hotPollPinned: true, + }), + ], + }) + ); // 3. Subsequent full poll reporting stale draft: true within 60s keeps pr.draft === false - // and preserves hot-set membership (checkStatus: "pending" passed to rebuildHotSets) + // and preserves hot-set membership (checkStatus: "running", hotPollPinned: true passed to rebuildHotSets) vi.mocked(pollService.rebuildHotSets).mockClear(); vi.mocked(pollService.fetchAllData).mockResolvedValue({ issues: [], @@ -4588,7 +4603,8 @@ describe("DashboardPage — Mark PR ready integration", () => { id: 77, nodeId: "PR_kwDOtest0077", enriched: true, - checkStatus: "pending", + checkStatus: "running", + hotPollPinned: true, }), ], }) @@ -5098,7 +5114,7 @@ describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets enriched: true, nodeId: "PR_kwDOtest0501", headSha: "1111111111111111111111111111111111111111", - checkStatus: "pending", + checkStatus: "running", reviewDecision: "REVIEW_REQUIRED", autoMergeMethod: "SQUASH", assigneeLogins: ["alice"], @@ -5159,7 +5175,7 @@ describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets id: 501, nodeId: "PR_kwDOtest0501", enriched: true, - checkStatus: "pending", + checkStatus: "running", autoMergeMethod: "SQUASH", assigneeLogins: ["alice"], reviewerLogins: ["bob"], @@ -5177,7 +5193,7 @@ describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets id: 501, nodeId: "PR_kwDOtest0501", enriched: true, - checkStatus: "pending", + checkStatus: "running", autoMergeMethod: "SQUASH", assigneeLogins: ["alice"], reviewerLogins: ["bob"], diff --git a/tests/lib/dependency-detection.test.ts b/tests/lib/dependency-detection.test.ts index fb03ada2..3e77e8d5 100644 --- a/tests/lib/dependency-detection.test.ts +++ b/tests/lib/dependency-detection.test.ts @@ -445,15 +445,17 @@ describe("classifyDepStatus", () => { expect(classifyDepStatus(pr, "", 14)).toBe("needs-action"); }); - it("returns needs-action for recent PR with pending CI", () => { - const pr = makePullRequest({ - enriched: true, - draft: false, - checkStatus: "pending", - reviewDecision: null, - updatedAt: RECENT, - }); - expect(classifyDepStatus(pr, "", 14)).toBe("needs-action"); + it("returns needs-action for recent PR with running, waiting, or pending CI", () => { + for (const checkStatus of ["running", "waiting", "pending"] as const) { + const pr = makePullRequest({ + enriched: true, + draft: false, + checkStatus, + reviewDecision: null, + updatedAt: RECENT, + }); + expect(classifyDepStatus(pr, "", 14)).toBe("needs-action"); + } }); it("returns needs-action for unenriched PR (enriched=false, draft=false, checkStatus is null)", () => { diff --git a/tests/lib/format.test.ts b/tests/lib/format.test.ts index 0a8af958..cc0f3fd7 100644 --- a/tests/lib/format.test.ts +++ b/tests/lib/format.test.ts @@ -578,7 +578,7 @@ describe("canApprovePR", () => { expect(canApprovePR(validPr, "")).toBe(false); }); - it("returns false when checkStatus is failure and true for success, pending, conflict, or null", () => { + it("returns false when checkStatus is failure and true for success, running, waiting, pending, conflict, or null", () => { const base = { nodeId: "PR_kwDOtest0001", headSha: validSha, @@ -588,6 +588,8 @@ describe("canApprovePR", () => { expect(canApprovePR(makePullRequest({ ...base, checkStatus: "failure" }), "reviewer")).toBe(false); expect(canApprovePR(makePullRequest({ ...base, checkStatus: "success" }), "reviewer")).toBe(true); + expect(canApprovePR(makePullRequest({ ...base, checkStatus: "running" }), "reviewer")).toBe(true); + expect(canApprovePR(makePullRequest({ ...base, checkStatus: "waiting" }), "reviewer")).toBe(true); expect(canApprovePR(makePullRequest({ ...base, checkStatus: "pending" }), "reviewer")).toBe(true); expect(canApprovePR(makePullRequest({ ...base, checkStatus: "conflict" }), "reviewer")).toBe(true); expect(canApprovePR(makePullRequest({ ...base, checkStatus: null }), "reviewer")).toBe(true); diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index cb414854..0f4481ca 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -2131,7 +2131,7 @@ describe("fetchPREnrichment — 25-node chunking, binary splitting, and light fa const octokit = makeOctokit( async () => ({ data: {} }), async (query: string) => { - expect(query).toContain("statusCheckRollup { state }"); + expect(query).toContain("statusCheckRollup { state contexts { checkRunCountsByState { state count } } }"); expect(query).not.toContain("commits(last: 1)"); return { nodes: [ @@ -2202,6 +2202,128 @@ describe("fetchPREnrichment — 25-node chunking, binary splitting, and light fa expect(e302!.reviewThreads).toBe(0); }); + it("classifies non-terminal statusCheckRollup into running, waiting, and pending via contexts.checkRunCountsByState and preserves all three under UNSTABLE mergeStateStatus", async () => { + const nodes = [ + // (a) IN_PROGRESS > 0 wins over WAITING > 0 -> "running" + buildHeavyNode(310, "PR_310", { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 2 }, + { state: "WAITING", count: 1 }, + ], + }, + }, + }), + // (b) IN_PROGRESS === 0, WAITING > 0 -> "waiting" (agent-director/platform #42 case) + buildHeavyNode(311, "PR_311", { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 0 }, + { state: "WAITING", count: 1 }, + { state: "SUCCESS", count: 11 }, + ], + }, + }, + }), + // (c) IN_PROGRESS === 0, WAITING === 0, QUEUED > 0 -> "pending" (osac-project/osac #2099 case) + buildHeavyNode(312, "PR_312", { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 0 }, + { state: "WAITING", count: 0 }, + { state: "QUEUED", count: 1 }, + { state: "SUCCESS", count: 13 }, + ], + }, + }, + }), + // (d) PENDING StatusContext (IN_PROGRESS: 0, WAITING: 0) + mergeStateStatus: "UNSTABLE" -> "pending" (not overridden to "failure") + buildHeavyNode(313, "PR_313", { + mergeStateStatus: "UNSTABLE", + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 0 }, + { state: "WAITING", count: 0 }, + ], + }, + }, + }), + // (e) Bare { state: "PENDING" } without contexts -> "pending" + buildHeavyNode(314, "PR_314", { + statusCheckRollup: { + state: "PENDING", + }, + }), + // (f1) IN_PROGRESS: 1 + mergeStateStatus: "UNSTABLE" -> "running" (not overridden to "failure") + buildHeavyNode(315, "PR_315", { + mergeStateStatus: "UNSTABLE", + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }], + }, + }, + }), + // (f2) WAITING: 1 + mergeStateStatus: "UNSTABLE" -> "waiting" (not overridden to "failure") + buildHeavyNode(316, "PR_316", { + mergeStateStatus: "UNSTABLE", + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [{ state: "WAITING", count: 1 }], + }, + }, + }), + // (f3) ACTION_REQUIRED > 0 in checkRunCountsByState under PENDING rollup -> "waiting" + buildHeavyNode(317, "PR_317", { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [{ state: "ACTION_REQUIRED", count: 1 }], + }, + }, + }), + // Control: terminal SUCCESS + mergeStateStatus: "UNSTABLE" -> overridden to "failure" + buildHeavyNode(318, "PR_318", { + mergeStateStatus: "UNSTABLE", + statusCheckRollup: { + state: "SUCCESS", + contexts: { + checkRunCountsByState: [{ state: "SUCCESS", count: 5 }], + }, + }, + }), + ]; + + const octokit = makeOctokit( + async () => ({ data: {} }), + async () => ({ nodes }), + ); + + const { enrichments, errors } = await fetchPREnrichment( + octokit as never, + nodes.map((n) => n.id), + ); + expect(errors).toEqual([]); + expect(enrichments.get(310)?.checkStatus).toBe("running"); + expect(enrichments.get(311)?.checkStatus).toBe("waiting"); + expect(enrichments.get(312)?.checkStatus).toBe("pending"); + expect(enrichments.get(313)?.checkStatus).toBe("pending"); + expect(enrichments.get(314)?.checkStatus).toBe("pending"); + expect(enrichments.get(315)?.checkStatus).toBe("running"); + expect(enrichments.get(316)?.checkStatus).toBe("waiting"); + expect(enrichments.get(317)?.checkStatus).toBe("waiting"); + expect(enrichments.get(318)?.checkStatus).toBe("failure"); + }); + it("excludes truncated node indices from err.errors[].path during partial salvage and retries 1..6 remaining IDs with HEAVY_PR_BACKFILL_QUERY", async () => { const ids = Array.from({ length: 9 }, (_, i) => `PR_${i + 1}`); const heavyCallIds: string[][] = []; @@ -2541,15 +2663,45 @@ describe("fetchIssuesAndPullRequests — settled-PR enrichment cache", () => { } }); - it("never caches PRs with checkStatus === 'pending' or checkStatusStale === true", async () => { + it("never caches PRs with non-terminal checkStatus ('running', 'waiting', 'pending') or checkStatusStale === true", async () => { let heavyCalls = 0; - let mode: "pending" | "stale" = "pending"; + let mode: "running" | "waiting" | "pending" | "stale" = "running"; const octokit = makeOctokit( async () => ({ data: {} }), async (query: string, variables?: unknown) => { const vars = (variables ?? {}) as Record; if ("ids" in vars) { + if (mode === "running") { + heavyCalls++; + return { + nodes: [ + makeHeavyPR({ + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }], + }, + }, + }), + ], + }; + } + if (mode === "waiting") { + heavyCalls++; + return { + nodes: [ + makeHeavyPR({ + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [{ state: "WAITING", count: 1 }], + }, + }, + }), + ], + }; + } if (mode === "pending") { heavyCalls++; return { @@ -2595,22 +2747,40 @@ describe("fetchIssuesAndPullRequests — settled-PR enrichment cache", () => { } ); + // Running CI status must not be cached + const runningRes1 = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + expect(runningRes1.pullRequests[0].checkStatus).toBe("running"); + expect(heavyCalls).toBe(1); + + await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + expect(heavyCalls).toBe(2); + + // Waiting CI status must not be cached + mode = "waiting"; + const waitingRes1 = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + expect(waitingRes1.pullRequests[0].checkStatus).toBe("waiting"); + expect(heavyCalls).toBe(3); + + await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + expect(heavyCalls).toBe(4); + // Pending CI status must not be cached + mode = "pending"; const pendingRes1 = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); expect(pendingRes1.pullRequests[0].checkStatus).toBe("pending"); - expect(heavyCalls).toBe(1); + expect(heavyCalls).toBe(5); await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); - expect(heavyCalls).toBe(2); + expect(heavyCalls).toBe(6); // Light fallback (checkStatusStale === true) must not be cached mode = "stale"; const staleRes1 = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); expect(staleRes1.pullRequests[0].checkStatusStale).toBe(true); - expect(heavyCalls).toBe(3); + expect(heavyCalls).toBe(7); await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); - expect(heavyCalls).toBe(4); + expect(heavyCalls).toBe(8); }); it("marks fork PRs with checkStatusStale === true and skips caching when runForkPRFallback fails", async () => { diff --git a/tests/services/events-poll.test.ts b/tests/services/events-poll.test.ts index 2af85df8..70b5ced3 100644 --- a/tests/services/events-poll.test.ts +++ b/tests/services/events-poll.test.ts @@ -307,11 +307,11 @@ describe("seedHotSetsFromTargeted", () => { vi.mocked(fetchWorkflowRunById).mockClear(); }); - it("adds enriched pending-checkStatus PRs with nodeId to hot set", async () => { + it("adds enriched running-checkStatus PRs with nodeId to hot set", async () => { seedHotSetsFromTargeted({ ...emptyData, pullRequests: [ - makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" }), + makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" }), ], }); @@ -356,7 +356,7 @@ describe("seedHotSetsFromTargeted", () => { seedHotSetsFromTargeted({ ...emptyData, pullRequests: [ - makePullRequest({ id: 3, checkStatus: "pending", enriched: false, nodeId: "PR_c" }), + makePullRequest({ id: 3, checkStatus: "running", enriched: false, nodeId: "PR_c" }), ], }); @@ -370,7 +370,7 @@ describe("seedHotSetsFromTargeted", () => { rebuildHotSets({ ...emptyData, pullRequests: [ - makePullRequest({ id: 10, checkStatus: "pending", enriched: true, nodeId: "PR_existing" }), + makePullRequest({ id: 10, checkStatus: "running", enriched: true, nodeId: "PR_existing" }), ], }); @@ -378,7 +378,7 @@ describe("seedHotSetsFromTargeted", () => { seedHotSetsFromTargeted({ ...emptyData, pullRequests: [ - makePullRequest({ id: 11, checkStatus: "pending", enriched: true, nodeId: "PR_new" }), + makePullRequest({ id: 11, checkStatus: "running", enriched: true, nodeId: "PR_new" }), ], }); @@ -396,7 +396,7 @@ describe("seedHotSetsFromTargeted", () => { seedHotSetsFromTargeted({ ...emptyData, pullRequests: [ - makePullRequest({ id: 20, checkStatus: "pending", enriched: true, nodeId: "PR_gen" }), + makePullRequest({ id: 20, checkStatus: "running", enriched: true, nodeId: "PR_gen" }), ], }); diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 03450fb8..929d8f5b 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -70,6 +70,7 @@ vi.mock("../../src/app/stores/auth", () => ({ import { resetPollState, rebuildHotSets, + seedHotSetsFromTargeted, clearHotSets, fetchHotData, createHotPollCoordinator, @@ -205,6 +206,89 @@ describe("fetchHotPRStatus", () => { expect(results.get(45)!.checkStatus).toBe("pending"); }); + it("classifies non-terminal statusCheckRollup into running, waiting, and pending and preserves all three when mergeStateStatus is UNSTABLE", async () => { + const octokit = makeOctokit(undefined, () => Promise.resolve({ + nodes: [ + { + databaseId: 451, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }] }, + }, + }, + { + databaseId: 452, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, + }, + { + databaseId: 453, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + }, + }, + { + databaseId: 454, + state: "OPEN", + mergeStateStatus: "UNSTABLE", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }] }, + }, + }, + { + databaseId: 455, + state: "OPEN", + mergeStateStatus: "UNSTABLE", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, + }, + { + databaseId: 456, + state: "OPEN", + mergeStateStatus: "UNSTABLE", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + }, + }, + ], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + + const { results } = await fetchHotPRStatus(octokit as never, [ + "PR_451", + "PR_452", + "PR_453", + "PR_454", + "PR_455", + "PR_456", + ]); + expect(results.get(451)!.checkStatus).toBe("running"); + expect(results.get(452)!.checkStatus).toBe("waiting"); + expect(results.get(453)!.checkStatus).toBe("pending"); + expect(results.get(454)!.checkStatus).toBe("running"); + expect(results.get(455)!.checkStatus).toBe("waiting"); + expect(results.get(456)!.checkStatus).toBe("pending"); + }); + it("maps headRefOid (headSha), autoMergeMethod, viewerLatestReviewState, and viewerReviewRequested", async () => { const validSha = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; const octokit = makeOctokit(undefined, () => Promise.resolve({ @@ -339,7 +423,7 @@ describe("fetchHotPRStatus", () => { expect(hadErrors).toBe(false); const calledQuery = octokit.graphql.mock.calls[0][0] as string; - expect(calledQuery).toContain("statusCheckRollup { state }"); + expect(calledQuery).toContain("statusCheckRollup { state contexts { checkRunCountsByState { state count } } }"); expect(calledQuery).toContain("updatedAt"); expect(calledQuery).not.toContain("commits(last: 1)"); @@ -401,7 +485,7 @@ describe("resetPollState", () => { it("clears hot sets and resets generation", async () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_x" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_x" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); expect(getHotPollGeneration()).toBe(1); @@ -430,7 +514,7 @@ describe("rebuildHotSets", () => { expect(getHotPollGeneration()).toBe(2); }); - it("populates hot PRs for enriched pending checkStatus with nodeId", async () => { + it("populates hot PRs for enriched running checkStatus with nodeId and skips waiting/pending without hotPollPinned", async () => { const octokit = makeOctokit(undefined, () => Promise.resolve({ nodes: [], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, @@ -440,11 +524,13 @@ describe("rebuildHotSets", () => { rebuildHotSets({ ...emptyData, pullRequests: [ - makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" }), - makePullRequest({ id: 2, checkStatus: null, enriched: true, nodeId: "PR_b" }), // null checkStatus — skipped (not pending) + makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" }), + makePullRequest({ id: 2, checkStatus: null, enriched: true, nodeId: "PR_b" }), // null checkStatus — skipped (not running) makePullRequest({ id: 3, checkStatus: "success", enriched: true, nodeId: "PR_c" }), // resolved — skipped - makePullRequest({ id: 4, checkStatus: "pending", enriched: true }), // no nodeId — skipped - makePullRequest({ id: 5, checkStatus: "pending", enriched: false, nodeId: "PR_e" }), // not enriched — skipped + makePullRequest({ id: 4, checkStatus: "running", enriched: true }), // no nodeId — skipped + makePullRequest({ id: 5, checkStatus: "running", enriched: false, nodeId: "PR_e" }), // not enriched — skipped + makePullRequest({ id: 6, checkStatus: "waiting", enriched: true, nodeId: "PR_waiting" }), // waiting without pin — skipped + makePullRequest({ id: 7, checkStatus: "pending", enriched: true, nodeId: "PR_pending" }), // pending without pin — skipped ], }); @@ -593,7 +679,7 @@ describe("fetchHotData", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_x" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_x" })], }); // First fetch — PR is hot, returns success -> evicts @@ -623,10 +709,10 @@ describe("fetchHotData", () => { const octokit = makeOctokit(undefined, graphqlFn); mockGetClient.mockReturnValue(octokit); - // Seed PR into _hotPRs (e.g. synthetically seeded with pending checkStatus before hot poll confirms no CI) + // Seed PR into _hotPRs (e.g. synthetically seeded with running checkStatus before hot poll confirms no CI) rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 2, checkStatus: "pending", enriched: true, nodeId: "PR_no_ci" })], + pullRequests: [makePullRequest({ id: 2, checkStatus: "running", enriched: true, nodeId: "PR_no_ci" })], }); // First fetch — PR is OPEN with checkStatus: null -> evicts after single cycle @@ -882,7 +968,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); const { pushError } = await import("../../src/app/lib/errors"); @@ -918,7 +1004,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); const Sentry = await import("@sentry/solid"); @@ -983,7 +1069,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 42, checkStatus: "pending", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], + pullRequests: [makePullRequest({ id: 42, checkStatus: "running", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], workflowRuns: [makeWorkflowRun({ id: 7, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -1068,7 +1154,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 42, checkStatus: "pending", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], + pullRequests: [makePullRequest({ id: 42, checkStatus: "running", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], }); const { pushError } = await import("../../src/app/lib/errors"); @@ -1091,7 +1177,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); const { expireToken } = await import("../../src/app/stores/auth"); @@ -1120,7 +1206,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); const graphqlFn = octokit.graphql as ReturnType; @@ -1261,13 +1347,74 @@ describe("fetchHotPRStatus edge cases", () => { const { results } = await fetchHotPRStatus(octokit as never, ["PR_61", "PR_62", "PR_63", "PR_64"]); const calledQuery = octokit.graphql.mock.calls[0][0] as string; expect(calledQuery).toContain("isCrossRepository"); - expect(calledQuery).toContain("headRef { target { ... on Commit { statusCheckRollup { state } } } }"); + expect(calledQuery).toContain("headRef { target { ... on Commit { statusCheckRollup { state contexts { checkRunCountsByState { state count } } } } } }"); expect(results.get(61)!.checkStatus).toBe("failure"); expect(results.get(62)!.checkStatus).toBe("success"); expect(results.get(63)!.checkStatus).toBeNull(); expect(results.get(64)!.checkStatus).toBeNull(); }); + it("classifies fork headRef.target.statusCheckRollup into running, waiting, and pending for cross-repository PRs", async () => { + const octokit = makeOctokit(undefined, () => Promise.resolve({ + nodes: [ + { + databaseId: 65, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: null, + isCrossRepository: true, + headRef: { + target: { + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }] }, + }, + }, + }, + }, + { + databaseId: 66, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: null, + isCrossRepository: true, + headRef: { + target: { + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, + }, + }, + }, + { + databaseId: 67, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: null, + isCrossRepository: true, + headRef: { + target: { + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + }, + }, + }, + }, + ], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + + const { results } = await fetchHotPRStatus(octokit as never, ["PR_65", "PR_66", "PR_67"]); + expect(results.get(65)!.checkStatus).toBe("running"); + expect(results.get(66)!.checkStatus).toBe("waiting"); + expect(results.get(67)!.checkStatus).toBe("pending"); + }); + it("salvages resolved nodes from a rejected batch's partial data and skips errored node indices", async () => { const octokit = makeOctokit(undefined, () => Promise.reject(Object.assign(new Error("Something went wrong while executing your query"), { @@ -1337,7 +1484,7 @@ describe("rebuildHotSets caps", () => { it("caps hot PRs at MAX_HOT_PRS (200)", async () => { const prs = Array.from({ length: 250 }, (_, i) => - makePullRequest({ id: i + 1, checkStatus: "pending", enriched: true, nodeId: `PR_${i}` }) + makePullRequest({ id: i + 1, checkStatus: "running", enriched: true, nodeId: `PR_${i}` }) ); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); @@ -1391,11 +1538,20 @@ describe("fetchHotData eviction edge cases", () => { const graphqlFn = vi.fn(() => { callCount++; if (callCount === 1) { - // First fetch: PR 1 resolved (success), PR 2 still pending + // First fetch: PR 1 resolved (success), PR 2 still running return Promise.resolve({ nodes: [ { databaseId: 1, state: "OPEN", mergeStateStatus: "CLEAN", reviewDecision: null, statusCheckRollup: { state: "SUCCESS" } }, - { databaseId: 2, state: "OPEN", mergeStateStatus: "CLEAN", reviewDecision: null, statusCheckRollup: { state: "PENDING" } }, + { + databaseId: 2, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }] }, + }, + }, ], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, }); @@ -1414,12 +1570,12 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, pullRequests: [ - makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_one" }), - makePullRequest({ id: 2, checkStatus: "pending", enriched: true, nodeId: "PR_two" }), + makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_one" }), + makePullRequest({ id: 2, checkStatus: "running", enriched: true, nodeId: "PR_two" }), ], }); - // First fetch — PR 1 resolves, PR 2 stays pending + // First fetch — PR 1 resolves, PR 2 stays running const first = await fetchHotData(); expect(first.prUpdates.size).toBe(2); @@ -1432,14 +1588,17 @@ describe("fetchHotData eviction edge cases", () => { expect(secondCallArgs[1].ids).toEqual(["PR_two"]); }); - it("evicts PRs when state is MERGED even with pending checkStatus", async () => { + it("evicts PRs when state is MERGED even with running checkStatus", async () => { const graphqlFn = vi.fn(() => Promise.resolve({ nodes: [{ databaseId: 1, state: "MERGED", mergeStateStatus: "CLEAN", reviewDecision: "APPROVED", - statusCheckRollup: { state: "PENDING" }, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 1 }] }, + }, }], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, })); @@ -1448,13 +1607,13 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_merged" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_merged" })], }); const first = await fetchHotData(); expect(first.prUpdates.get(1)!.state).toBe("MERGED"); - // Should be evicted — MERGED state takes priority over pending checks + // Should be evicted — MERGED state takes priority over running checks graphqlFn.mockClear(); const second = await fetchHotData(); expect(second.prUpdates.size).toBe(0); @@ -1477,7 +1636,7 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 2, checkStatus: "pending", enriched: true, nodeId: "PR_closed" })], + pullRequests: [makePullRequest({ id: 2, checkStatus: "running", enriched: true, nodeId: "PR_closed" })], }); await fetchHotData(); @@ -1485,13 +1644,211 @@ describe("fetchHotData eviction edge cases", () => { const second = await fetchHotData(); expect(second.prUpdates.size).toBe(0); }); + + it("evicts a running PR from _hotPRs when a hot-poll cycle transitions it to waiting or pending", async () => { + const graphqlFn = vi.fn(() => Promise.resolve({ + nodes: [ + { + databaseId: 10, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, + }, + { + databaseId: 11, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + }, + }, + ], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + const octokit = makeOctokit(undefined, graphqlFn); + mockGetClient.mockReturnValue(octokit); + + rebuildHotSets({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 10, checkStatus: "running", enriched: true, nodeId: "PR_to_waiting" }), + makePullRequest({ id: 11, checkStatus: "running", enriched: true, nodeId: "PR_to_pending" }), + ], + }); + + const first = await fetchHotData(); + expect(first.prUpdates.get(10)!.checkStatus).toBe("waiting"); + expect(first.prUpdates.get(11)!.checkStatus).toBe("pending"); + + graphqlFn.mockClear(); + const second = await fetchHotData(); + expect(second.prUpdates.size).toBe(0); + expect(graphqlFn).not.toHaveBeenCalled(); + }); + + it("evicts an already-enrolled unpinned running PR when seedHotSetsFromTargeted observes it transition to waiting, pending, or success while preserving hot PRs in other repos", async () => { + const octokit = makeOctokit(undefined, () => Promise.resolve({ + nodes: [], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + mockGetClient.mockReturnValue(octokit); + + rebuildHotSets({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_other_repo" }), + makePullRequest({ id: 2, checkStatus: "running", enriched: true, nodeId: "PR_now_waiting" }), + makePullRequest({ id: 3, checkStatus: "running", enriched: true, nodeId: "PR_now_pending" }), + makePullRequest({ id: 4, checkStatus: "running", enriched: true, nodeId: "PR_now_success" }), + ], + }); + + // Also verify seedHotSetsFromTargeted does NOT enroll brand-new waiting/pending PRs without hotPollPinned + seedHotSetsFromTargeted({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 2, checkStatus: "waiting", enriched: true, nodeId: "PR_now_waiting" }), + makePullRequest({ id: 3, checkStatus: "pending", enriched: true, nodeId: "PR_now_pending" }), + makePullRequest({ id: 4, checkStatus: "success", enriched: true, nodeId: "PR_now_success" }), + makePullRequest({ id: 5, checkStatus: "waiting", enriched: true, nodeId: "PR_new_waiting" }), + makePullRequest({ id: 6, checkStatus: "pending", enriched: true, nodeId: "PR_new_pending" }), + ], + }); + + await fetchHotData(); + expect(octokit.graphql).toHaveBeenCalledTimes(1); + const calledIds = (octokit.graphql.mock.calls[0][1] as { ids: string[] }).ids; + expect(calledIds).toEqual(["PR_other_repo"]); + }); + + it("evicts a workflow run from _hotRuns when it transitions from in_progress to waiting on a hot-poll cycle or via seedHotSetsFromTargeted", async () => { + const requestFn = vi.fn(() => Promise.resolve({ + data: { id: 10, status: "waiting", conclusion: null, updated_at: "2026-01-01T00:02:00Z", completed_at: null }, + headers: {}, + })); + const octokit = makeOctokit(requestFn); + mockGetClient.mockReturnValue(octokit); + + rebuildHotSets({ + ...emptyData, + workflowRuns: [ + makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" }), + makeWorkflowRun({ id: 11, status: "in_progress", conclusion: null, repoFullName: "o/r" }), + ], + }); + + // seedHotSetsFromTargeted observes run 11 transition to waiting -> evicted before fetchHotData + seedHotSetsFromTargeted({ + ...emptyData, + workflowRuns: [ + makeWorkflowRun({ id: 11, status: "waiting", conclusion: null, repoFullName: "o/r" }), + ], + }); + + // First fetchHotData queries only run 10, which returns status: "waiting" -> evicted + const first = await fetchHotData(); + expect(requestFn).toHaveBeenCalledTimes(1); + expect(first.runUpdates.get(10)!.status).toBe("waiting"); + + requestFn.mockClear(); + const second = await fetchHotData(); + expect(second.runUpdates.size).toBe(0); + expect(requestFn).not.toHaveBeenCalled(); + }); + + it("retains a hotPollPinned PR in _hotPRs across hot-poll ticks returning pending or success within 60s and evicts once the 60s pin expires or when unpinned", async () => { + vi.useFakeTimers(); + try { + let statusState: "PENDING" | "SUCCESS" = "PENDING"; + const graphqlFn = vi.fn(() => Promise.resolve({ + nodes: [{ + databaseId: 77, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: "APPROVED", + statusCheckRollup: { state: statusState }, + }], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + const octokit = makeOctokit(undefined, graphqlFn); + mockGetClient.mockReturnValue(octokit); + + // Seed PR 77 with hotPollPinned: true + seedHotSetsFromTargeted({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 77, checkStatus: "running", hotPollPinned: true, enriched: true, nodeId: "PR_pinned" }), + ], + }); + + // Tick 1 at +15s: returns checkStatus: "pending" -> retained because 60s pin is still active + vi.advanceTimersByTime(15_000); + const first = await fetchHotData(); + expect(first.prUpdates.get(77)!.checkStatus).toBe("pending"); + + // Tick 2 at +30s: returns checkStatus: "success" -> retained because 60s pin is still active + statusState = "SUCCESS"; + vi.advanceTimersByTime(15_000); + const second = await fetchHotData(); + expect(second.prUpdates.get(77)!.checkStatus).toBe("success"); + + // Advance past the 60s pin window (+31s -> 61s total): next tick evicts PR 77 + vi.advanceTimersByTime(31_000); + const third = await fetchHotData(); + expect(third.prUpdates.get(77)!.checkStatus).toBe("success"); + + graphqlFn.mockClear(); + const fourth = await fetchHotData(); + expect(fourth.prUpdates.size).toBe(0); + expect(graphqlFn).not.toHaveBeenCalled(); + + // Re-pin PR 77, then run seedHotSetsFromTargeted / rebuildHotSets without hotPollPinned -> pin dropped immediately + seedHotSetsFromTargeted({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 77, checkStatus: "running", hotPollPinned: true, enriched: true, nodeId: "PR_pinned" }), + ], + }); + seedHotSetsFromTargeted({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 77, checkStatus: "success", enriched: true, nodeId: "PR_pinned" }), + ], + }); + const afterTargetedUnpin = await fetchHotData(); + expect(afterTargetedUnpin.prUpdates.size).toBe(0); + + rebuildHotSets({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 77, checkStatus: "running", hotPollPinned: true, enriched: true, nodeId: "PR_pinned" }), + ], + }); + rebuildHotSets({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 77, checkStatus: "success", enriched: true, nodeId: "PR_pinned" }), + ], + }); + const afterRebuildUnpin = await fetchHotData(); + expect(afterRebuildUnpin.prUpdates.size).toBe(0); + } finally { + vi.useRealTimers(); + } + }); }); describe("clearHotSets", () => { it("empties both hot maps so next fetchHotData is a no-op", async () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -1529,7 +1886,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -1543,7 +1900,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); const { hadErrors, prUpdates } = await fetchHotData(); @@ -1575,7 +1932,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); await expect(fetchHotData()).rejects.toMatchObject({ status: 401 }); @@ -1602,7 +1959,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); await fetchHotData(2); // 2 prior failures — still below the threshold of 3 @@ -1618,7 +1975,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "running", enriched: true, nodeId: "PR_a" })], }); await fetchHotData(3); // 3 prior failures — meets the threshold From e97c8b63a3129cc1144d114174f89cad80913930 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 21:45:38 -0400 Subject: [PATCH 02/10] feat(ui): surface running, pending action, and pending condition checks Update StatusDot STATUS_CONFIG with distinct 'running', 'waiting', and 'pending' styles and tooltips. Expand PullRequestFiltersSchema.checkStatus and ActionsFiltersSchema.conclusion in view.ts and filterTypes.ts. Render 'Checks running', 'Pending action', and 'Pending condition' badges across DependenciesTab, PullRequestsTab, and TrackedTab, and update checkStatusOrder and repo-group check counts. --- .../components/dashboard/DependenciesTab.tsx | 12 +- .../components/dashboard/PullRequestsTab.tsx | 40 ++++- src/app/components/dashboard/TrackedTab.tsx | 12 ++ src/app/components/shared/StatusDot.tsx | 6 +- src/app/components/shared/filterTypes.ts | 8 +- src/app/stores/view.ts | 4 +- tests/components/DashboardPage.test.tsx | 2 +- tests/components/PullRequestsTab.test.tsx | 160 ++++++++++++++++++ .../dashboard/DependenciesTab.test.tsx | 38 ++++- .../dashboard/PullRequestsTab.test.tsx | 73 ++++++++ .../components/dashboard/TrackedTab.test.tsx | 73 ++++++++ tests/components/shared/StatusDot.test.tsx | 31 +++- 12 files changed, 434 insertions(+), 25 deletions(-) diff --git a/src/app/components/dashboard/DependenciesTab.tsx b/src/app/components/dashboard/DependenciesTab.tsx index 6ec6e1ff..90e1c1fe 100644 --- a/src/app/components/dashboard/DependenciesTab.tsx +++ b/src/app/components/dashboard/DependenciesTab.tsx @@ -4,7 +4,7 @@ import { viewState, setTabFilter, resetAllTabFilters, ignoreItem, unignoreItem, import IgnoreBadge from "./IgnoreBadge"; import { isSafeGitHubUrl } from "../../lib/url"; import { canApprovePR, canMarkPRReady } from "../../lib/format"; -import type { PullRequest } from "../../services/api"; +import { isNonTerminalCheckStatus, type PullRequest } from "../../services/api"; import type { AbandonedDependency } from "../../lib/dependency-dashboard"; import { classifyDepStatus, @@ -491,13 +491,19 @@ function StatusGroup(props: StatusGroupProps) { Failing checks - + Checks running + + Pending action + + + Pending condition + diff --git a/src/app/components/dashboard/PullRequestsTab.tsx b/src/app/components/dashboard/PullRequestsTab.tsx index 434e5b27..7ea12e1a 100644 --- a/src/app/components/dashboard/PullRequestsTab.tsx +++ b/src/app/components/dashboard/PullRequestsTab.tsx @@ -61,12 +61,16 @@ function checkStatusOrder(status: PullRequest["checkStatus"]): number { return 0; case "conflict": return 1; - case "pending": + case "waiting": return 2; - case "success": + case "running": return 3; - default: + case "pending": return 4; + case "success": + return 5; + default: + return 6; } } @@ -385,13 +389,15 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup.repoFullName); const summaryMeta = createMemo(() => { - const checks = { success: 0, failure: 0, pending: 0, conflict: 0 }; + const checks = { success: 0, failure: 0, running: 0, waiting: 0, pending: 0, conflict: 0 }; const reviews = { APPROVED: 0, CHANGES_REQUESTED: 0, REVIEW_REQUIRED: 0 }; const roles: Record = {}; for (const item of repoGroup.items) { if (item.checkStatus === "success") checks.success++; else if (item.checkStatus === "failure") checks.failure++; + else if (item.checkStatus === "running") checks.running++; + else if (item.checkStatus === "waiting") checks.waiting++; else if (item.checkStatus === "pending") checks.pending++; else if (item.checkStatus === "conflict") checks.conflict++; @@ -452,9 +458,21 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { {summaryMeta().checks.failure} - 0}> + 0}> + {summaryMeta().checks.running} + + + 0}> + + + {summaryMeta().checks.waiting} + + + 0}> + + {summaryMeta().checks.pending} @@ -563,6 +581,12 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { Merge conflict + + Pending action + + + Pending condition + + + Pending action + + + Pending condition + + + Pending action + + + Pending condition + + + Pending action + + + Pending condition + { it("discards stale hot poll updates when generation mismatches", async () => { const testPR = makePullRequest({ id: 43, - checkStatus: "pending", + checkStatus: "running", state: "OPEN", }); vi.mocked(pollService.fetchAllData).mockResolvedValue({ diff --git a/tests/components/PullRequestsTab.test.tsx b/tests/components/PullRequestsTab.test.tsx index 40e41464..a4269fe5 100644 --- a/tests/components/PullRequestsTab.test.tsx +++ b/tests/components/PullRequestsTab.test.tsx @@ -899,4 +899,164 @@ describe("PullRequestsTab", () => { expect(onMarkPRReady).toHaveBeenCalledWith(ownDraftPR); }); }); + + describe("acute checkStatus states (running, waiting, pending)", () => { + it("renders 'Pending action' and 'Pending condition' row badges in comfortable and compact density when enriched, and omits when unenriched", () => { + setAllExpanded("pullRequests", true); + const runningPR = makePullRequest({ + id: 30, + number: 30, + title: "Running PR", + repoFullName: "org/repo", + checkStatus: "running", + enriched: true, + }); + const waitingPR = makePullRequest({ + id: 31, + number: 31, + title: "Waiting PR", + repoFullName: "org/repo", + checkStatus: "waiting", + enriched: true, + }); + const pendingPR = makePullRequest({ + id: 32, + number: 32, + title: "Pending Condition PR", + repoFullName: "org/repo", + checkStatus: "pending", + enriched: true, + }); + const unenrichedWaitingPR = makePullRequest({ + id: 33, + number: 33, + title: "Unenriched Waiting PR", + repoFullName: "org/repo", + checkStatus: "waiting", + enriched: false, + }); + + const { unmount } = render(() => ( + + )); + + expect(screen.getByLabelText("Checks in progress")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending human action")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending condition")).toBeTruthy(); + + const waitingSpans = screen + .getAllByText("Pending action") + .filter((el) => el.tagName.toLowerCase() === "span"); + expect(waitingSpans).toHaveLength(1); + expect(waitingSpans[0].className).toContain("badge-warning"); + + const pendingSpans = screen + .getAllByText("Pending condition") + .filter((el) => el.tagName.toLowerCase() === "span"); + expect(pendingSpans).toHaveLength(1); + expect(pendingSpans[0].className).toContain("badge-ghost"); + unmount(); + + // Compact density + setConfig("viewDensity", "compact"); + render(() => ( + + )); + + const compactWaitingSpans = screen + .getAllByText("Pending action") + .filter((el) => el.tagName.toLowerCase() === "span"); + expect(compactWaitingSpans).toHaveLength(1); + + const compactPendingSpans = screen + .getAllByText("Pending condition") + .filter((el) => el.tagName.toLowerCase() === "span"); + expect(compactPendingSpans).toHaveLength(1); + }); + + it("sorts PRs by checkStatusOrder: failure < conflict < waiting < running < pending < success < null", () => { + setAllExpanded("pullRequests", true); + viewStore.setSortPreference("checkStatus", "asc"); + const prs = [ + makePullRequest({ id: 1, title: "Null CI PR", checkStatus: null, repoFullName: "org/repo" }), + makePullRequest({ id: 2, title: "Success PR", checkStatus: "success", repoFullName: "org/repo" }), + makePullRequest({ id: 3, title: "Pending PR", checkStatus: "pending", repoFullName: "org/repo" }), + makePullRequest({ id: 4, title: "Running PR", checkStatus: "running", repoFullName: "org/repo" }), + makePullRequest({ id: 5, title: "Waiting PR", checkStatus: "waiting", repoFullName: "org/repo" }), + makePullRequest({ id: 6, title: "Conflict PR", checkStatus: "conflict", repoFullName: "org/repo" }), + makePullRequest({ id: 7, title: "Failure PR", checkStatus: "failure", repoFullName: "org/repo" }), + ]; + + render(() => ); + const items = screen.getAllByRole("listitem").map((el) => el.textContent ?? ""); + + const idx = (title: string) => items.findIndex((t) => t.includes(title)); + expect(idx("Failure PR")).toBeLessThan(idx("Conflict PR")); + expect(idx("Conflict PR")).toBeLessThan(idx("Waiting PR")); + expect(idx("Waiting PR")).toBeLessThan(idx("Running PR")); + expect(idx("Running PR")).toBeLessThan(idx("Pending PR")); + expect(idx("Pending PR")).toBeLessThan(idx("Success PR")); + expect(idx("Success PR")).toBeLessThan(idx("Null CI PR")); + }); + + it("shows running, waiting, and pending check counts and indicator dots in collapsed repo summary", () => { + const prs = [ + makePullRequest({ id: 1, title: "Run 1", checkStatus: "running", repoFullName: "org/repo-a" }), + makePullRequest({ id: 2, title: "Run 2", checkStatus: "running", repoFullName: "org/repo-a" }), + makePullRequest({ id: 3, title: "Wait 1", checkStatus: "waiting", repoFullName: "org/repo-a" }), + makePullRequest({ id: 4, title: "Wait 2", checkStatus: "waiting", repoFullName: "org/repo-a" }), + makePullRequest({ id: 5, title: "Wait 3", checkStatus: "waiting", repoFullName: "org/repo-a" }), + makePullRequest({ id: 6, title: "Pend 1", checkStatus: "pending", repoFullName: "org/repo-a" }), + ]; + + render(() => ); + const header = screen.getByText("org/repo-a").closest("button")!; + const summarySpan = header.querySelector(".ml-auto")!; + + const runningDot = summarySpan.querySelector(".bg-warning"); + expect(runningDot).not.toBeNull(); + expect(runningDot?.parentElement?.textContent).toBe("2"); + + const waitingDot = summarySpan.querySelector(".bg-amber-500"); + expect(waitingDot).not.toBeNull(); + expect(waitingDot?.parentElement?.textContent).toBe("3"); + + const pendingDot = summarySpan.querySelector(".border-warning\\/80"); + expect(pendingDot).not.toBeNull(); + expect(pendingDot?.parentElement?.textContent).toBe("1"); + }); + + it("filters PRs by 'running', 'waiting', and 'pending' checkStatus tab filter", () => { + setAllExpanded("pullRequests", true); + const prs = [ + makePullRequest({ id: 1, title: "Running PR", checkStatus: "running", repoFullName: "org/repo-a" }), + makePullRequest({ id: 2, title: "Waiting PR", checkStatus: "waiting", repoFullName: "org/repo-a" }), + makePullRequest({ id: 3, title: "Pending PR", checkStatus: "pending", repoFullName: "org/repo-a" }), + makePullRequest({ id: 4, title: "Passing PR", checkStatus: "success", repoFullName: "org/repo-a" }), + ]; + + viewStore.setTabFilter("pullRequests", "checkStatus", "running"); + render(() => ); + + expect(screen.getByText("Running PR")).toBeTruthy(); + expect(screen.queryByText("Waiting PR")).toBeNull(); + expect(screen.queryByText("Pending PR")).toBeNull(); + expect(screen.queryByText("Passing PR")).toBeNull(); + + viewStore.setTabFilter("pullRequests", "checkStatus", "waiting"); + expect(screen.queryByText("Running PR")).toBeNull(); + expect(screen.getByText("Waiting PR")).toBeTruthy(); + expect(screen.queryByText("Pending PR")).toBeNull(); + + viewStore.setTabFilter("pullRequests", "checkStatus", "pending"); + expect(screen.queryByText("Waiting PR")).toBeNull(); + expect(screen.getByText("Pending PR")).toBeTruthy(); + }); + }); }); diff --git a/tests/components/dashboard/DependenciesTab.test.tsx b/tests/components/dashboard/DependenciesTab.test.tsx index 097eb751..cdbd90a7 100644 --- a/tests/components/dashboard/DependenciesTab.test.tsx +++ b/tests/components/dashboard/DependenciesTab.test.tsx @@ -939,7 +939,7 @@ describe("DependenciesTab — auto-merge badge and Approve action", () => { expect(onMarkPRReady).toHaveBeenCalledWith(selfDraftDepPR); }); - it("renders 'Merge conflict', 'Failing checks', and 'Checks running' badges in 'Needs Action' and links StatusDot to /checks", () => { + it("renders 'Merge conflict', 'Failing checks', 'Checks running', 'Pending action', and 'Pending condition' badges in 'Needs Action' and links StatusDot to /checks", () => { setDependencyExpandedGroups(["mergeable", "needs-action"]); const conflictPR = makeNeedsActionPR({ id: 8030, @@ -957,23 +957,45 @@ describe("DependenciesTab — auto-merge badge and Approve action", () => { checkStatus: "failure", enriched: true, }); - const pendingPR = makeNeedsActionPR({ + const runningPR = makeNeedsActionPR({ id: 8032, number: 832, title: "Bump react from 17.0.0 to 18.0.0", htmlUrl: "https://github.com/owner/repo/pull/832", + checkStatus: "running", + checkStatusStale: true, + enriched: true, + }); + const waitingPR = makeNeedsActionPR({ + id: 8033, + number: 833, + title: "Bump vue from 3.2.0 to 3.3.0", + htmlUrl: "https://github.com/owner/repo/pull/833", + checkStatus: "waiting", + checkStatusStale: true, + enriched: true, + }); + const pendingConditionPR = makeNeedsActionPR({ + id: 8034, + number: 834, + title: "Bump svelte from 3.0.0 to 4.0.0", + htmlUrl: "https://github.com/owner/repo/pull/834", checkStatus: "pending", + checkStatusStale: true, enriched: true, }); renderTab({ - pullRequests: [conflictPR, failingPR, pendingPR], + pullRequests: [conflictPR, failingPR, runningPR, waitingPR, pendingConditionPR], userLogin: "testuser", }); expect(screen.getByText("Merge conflict")).toBeDefined(); expect(screen.getByText("Failing checks")).toBeDefined(); expect(screen.getByText("Checks running")).toBeDefined(); + expect(screen.getByText("Pending action")).toBeDefined(); + expect(screen.getByText("Pending condition")).toBeDefined(); + expect(screen.queryByText("Checking status")).toBeNull(); const conflictDotLink = screen.getByLabelText("Checks blocked by merge conflict").closest("a"); expect(conflictDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/830/checks"); @@ -981,8 +1003,14 @@ describe("DependenciesTab — auto-merge badge and Approve action", () => { const failingDotLink = screen.getByLabelText("Checks failing").closest("a"); expect(failingDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/831/checks"); - const pendingDotLink = screen.getByLabelText("Checks in progress").closest("a"); - expect(pendingDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/832/checks"); + const runningDotLink = screen.getByLabelText("Checks in progress").closest("a"); + expect(runningDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/832/checks"); + + const waitingDotLink = screen.getByLabelText("Checks pending human action").closest("a"); + expect(waitingDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/833/checks"); + + const pendingConditionDotLink = screen.getByLabelText("Checks pending condition").closest("a"); + expect(pendingConditionDotLink?.getAttribute("href")).toBe("https://github.com/owner/repo/pull/834/checks"); }); it("classifies enriched non-draft PRs with checkStatus: null or reviewDecision: 'APPROVED' into 'Mergeable' and 'CHANGES_REQUESTED' into 'Needs Action'", () => { diff --git a/tests/components/dashboard/PullRequestsTab.test.tsx b/tests/components/dashboard/PullRequestsTab.test.tsx index ce8d39f4..c28c2edb 100644 --- a/tests/components/dashboard/PullRequestsTab.test.tsx +++ b/tests/components/dashboard/PullRequestsTab.test.tsx @@ -900,3 +900,76 @@ describe("PullRequestsTab — depPrIds ignored-item filtering", () => { expect(screen.queryByRole("button", { name: /ignored/i })).toBeNull(); }); }); + +// ── PullRequestsTab — running, waiting, and pending check states ──────────── + +describe("PullRequestsTab — running, waiting, and pending check states", () => { + it("renders 'Pending action' and 'Pending condition' badges in comfortable and compact density modes", () => { + setAllExpanded("pullRequests", true); + const prs = [ + makePullRequest({ id: 1, title: "Running PR", repoFullName: "org/repo", checkStatus: "running", surfacedBy: ["me"], enriched: true }), + makePullRequest({ id: 2, title: "Waiting PR", repoFullName: "org/repo", checkStatus: "waiting", surfacedBy: ["me"], enriched: true }), + makePullRequest({ id: 3, title: "Pending PR", repoFullName: "org/repo", checkStatus: "pending", surfacedBy: ["me"], enriched: true }), + ]; + + const { unmount } = render(() => ( + + )); + + expect(screen.getByLabelText("Checks in progress")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending human action")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending condition")).toBeTruthy(); + + const waitingBadge = screen.getAllByText("Pending action").find((el) => el.tagName.toLowerCase() === "span"); + expect(waitingBadge).toBeDefined(); + expect(waitingBadge?.className).toContain("badge-warning"); + + const pendingBadge = screen.getAllByText("Pending condition").find((el) => el.tagName.toLowerCase() === "span"); + expect(pendingBadge).toBeDefined(); + expect(pendingBadge?.className).toContain("badge-ghost"); + unmount(); + + updateConfig({ viewDensity: "compact" }); + render(() => ( + + )); + + expect(screen.getAllByText("Pending action").find((el) => el.tagName.toLowerCase() === "span")).toBeDefined(); + expect(screen.getAllByText("Pending condition").find((el) => el.tagName.toLowerCase() === "span")).toBeDefined(); + }); + + it("filters PRs by checkStatus=running, checkStatus=waiting, and checkStatus=pending", () => { + setAllExpanded("pullRequests", true); + const prs = [ + makePullRequest({ id: 1, title: "Running PR", repoFullName: "org/repo", checkStatus: "running", surfacedBy: ["me"], enriched: true }), + makePullRequest({ id: 2, title: "Waiting PR", repoFullName: "org/repo", checkStatus: "waiting", surfacedBy: ["me"], enriched: true }), + makePullRequest({ id: 3, title: "Pending PR", repoFullName: "org/repo", checkStatus: "pending", surfacedBy: ["me"], enriched: true }), + ]; + + setTabFilter("pullRequests", "checkStatus", "running"); + const { unmount } = render(() => ( + + )); + screen.getByText("Running PR"); + expect(screen.queryByText("Waiting PR")).toBeNull(); + expect(screen.queryByText("Pending PR")).toBeNull(); + unmount(); + + setTabFilter("pullRequests", "checkStatus", "waiting"); + const { unmount: unmount2 } = render(() => ( + + )); + expect(screen.queryByText("Running PR")).toBeNull(); + screen.getByText("Waiting PR"); + expect(screen.queryByText("Pending PR")).toBeNull(); + unmount2(); + + setTabFilter("pullRequests", "checkStatus", "pending"); + render(() => ( + + )); + expect(screen.queryByText("Running PR")).toBeNull(); + expect(screen.queryByText("Waiting PR")).toBeNull(); + screen.getByText("Pending PR"); + }); +}); diff --git a/tests/components/dashboard/TrackedTab.test.tsx b/tests/components/dashboard/TrackedTab.test.tsx index 27c03d17..b82b8e27 100644 --- a/tests/components/dashboard/TrackedTab.test.tsx +++ b/tests/components/dashboard/TrackedTab.test.tsx @@ -562,4 +562,77 @@ describe("TrackedTab — auto-merge badge and Approve action", () => { expect(onMarkPRReady).toHaveBeenCalledOnce(); expect(onMarkPRReady).toHaveBeenCalledWith(ownDraftPR); }); + + it("renders 'Pending action' and 'Pending condition' badges and StatusDot labels on tracked PR rows in comfortable and compact density", () => { + const runningPR = makePullRequest({ + id: 940, + number: 940, + title: "Running Tracked PR", + checkStatus: "running", + enriched: true, + }); + const waitingPR = makePullRequest({ + id: 941, + number: 941, + title: "Waiting Tracked PR", + checkStatus: "waiting", + enriched: true, + }); + const pendingPR = makePullRequest({ + id: 942, + number: 942, + title: "Pending Tracked PR", + checkStatus: "pending", + enriched: true, + }); + const unenrichedWaitingPR = makePullRequest({ + id: 943, + number: 943, + title: "Unenriched Waiting Tracked PR", + checkStatus: "waiting", + enriched: false, + }); + + updateViewState({ + trackedItems: [ + makeTrackedItem({ id: 940, number: 940, type: "pullRequest", title: "Running Tracked PR" }), + makeTrackedItem({ id: 941, number: 941, type: "pullRequest", title: "Waiting Tracked PR" }), + makeTrackedItem({ id: 942, number: 942, type: "pullRequest", title: "Pending Tracked PR" }), + makeTrackedItem({ id: 943, number: 943, type: "pullRequest", title: "Unenriched Waiting Tracked PR" }), + ], + }); + + const { unmount } = render(() => ( + + )); + + expect(screen.getByLabelText("Checks in progress")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending human action")).toBeTruthy(); + expect(screen.getByLabelText("Checks pending condition")).toBeTruthy(); + + const waitingBadges = screen.getAllByText("Pending action"); + expect(waitingBadges).toHaveLength(1); + expect(waitingBadges[0].className).toContain("badge-warning"); + + const pendingBadges = screen.getAllByText("Pending condition"); + expect(pendingBadges).toHaveLength(1); + expect(pendingBadges[0].className).toContain("badge-ghost"); + unmount(); + + setConfig("viewDensity", "compact"); + render(() => ( + + )); + + expect(screen.getAllByText("Pending action")).toHaveLength(1); + expect(screen.getAllByText("Pending condition")).toHaveLength(1); + }); }); diff --git a/tests/components/shared/StatusDot.test.tsx b/tests/components/shared/StatusDot.test.tsx index 4741ead6..1f0bfea7 100644 --- a/tests/components/shared/StatusDot.test.tsx +++ b/tests/components/shared/StatusDot.test.tsx @@ -9,10 +9,27 @@ describe("StatusDot", () => { expect(wrapper?.getAttribute("aria-label")).toBe("All checks passed"); }); - it('shows "Checks in progress" label for status="pending"', () => { - const { container } = render(() => ); + it('shows "Checks in progress" label and bg-warning for status="running"', () => { + const { container } = render(() => ); const wrapper = container.querySelector("[aria-label]"); expect(wrapper?.getAttribute("aria-label")).toBe("Checks in progress"); + expect(container.querySelector(".bg-warning")).not.toBeNull(); + }); + + it('shows "Checks pending human action" label and bg-amber-500 for status="waiting"', () => { + const { container } = render(() => ); + const wrapper = container.querySelector("[aria-label]"); + expect(wrapper?.getAttribute("aria-label")).toBe("Checks pending human action"); + expect(container.querySelector(".bg-amber-500")).not.toBeNull(); + }); + + it('shows "Checks pending condition" label and ringed warning style for status="pending"', () => { + const { container } = render(() => ); + const wrapper = container.querySelector("[aria-label]"); + expect(wrapper?.getAttribute("aria-label")).toBe("Checks pending condition"); + const dot = wrapper?.querySelector("span"); + expect(dot?.className).toContain("border-warning/80"); + expect(dot?.className).toContain("bg-warning/20"); }); it('shows "Checks failing" label for status="failure"', () => { @@ -50,9 +67,15 @@ describe("StatusDot", () => { expect(link!.getAttribute("rel")).toBe("noopener noreferrer"); }); - it("has animate-slow-pulse class only for status=pending", () => { + it("has animate-slow-pulse class only for status=running", () => { + const { container: runningContainer } = render(() => ); + expect(runningContainer.querySelector(".animate-slow-pulse")).not.toBeNull(); + + const { container: waitingContainer } = render(() => ); + expect(waitingContainer.querySelector(".animate-slow-pulse")).toBeNull(); + const { container: pendingContainer } = render(() => ); - expect(pendingContainer.querySelector(".animate-slow-pulse")).not.toBeNull(); + expect(pendingContainer.querySelector(".animate-slow-pulse")).toBeNull(); const { container: successContainer } = render(() => ); expect(successContainer.querySelector(".animate-slow-pulse")).toBeNull(); From 95f609475c969f446373f341287e5e9b6e40dbe6 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 21:51:31 -0400 Subject: [PATCH 03/10] feat(actions): surface pending action and condition across ActionsTab Export isWorkflowRunWaiting and isWorkflowRunPending in filters.ts and surface 'Pending action' and 'Pending condition' across WorkflowRunRow, WorkflowSummaryCard, ActionsTab (sortWorkflowsByStatus, filteredRuns, workflowCounts, and collapsedSummary), and DashboardPage custom-tab action badge counts. Update docs/USER_GUIDE.md and README.md. --- README.md | 8 +-- docs/USER_GUIDE.md | 20 ++++--- src/app/components/dashboard/ActionsTab.tsx | 45 ++++++++++---- .../components/dashboard/DashboardPage.tsx | 4 +- .../components/dashboard/WorkflowRunRow.tsx | 27 ++++++++- .../dashboard/WorkflowSummaryCard.tsx | 27 ++++++++- src/app/lib/filters.ts | 12 ++++ tests/components/ActionsTab.test.tsx | 59 +++++++++++++++++++ tests/components/DashboardPage.test.tsx | 44 ++++++++++++++ tests/components/WorkflowRunRow.test.tsx | 48 ++++++++++++++- tests/components/WorkflowSummaryCard.test.tsx | 39 ++++++++++++ 11 files changed, 302 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 67db53bc..c0d41e91 100644 --- a/README.md +++ b/README.md @@ -33,11 +33,11 @@ Open issues where you're the creator, assignee, or mentioned. A scope filter let ### Pull Requests -Open PRs with CI status dots (green/yellow/red), review decision badges, a read-only `auto` badge when GitHub auto-merge is enabled (with merge method tooltip), size badges (XS–XL by lines changed), clickable two-step `Draft` → `Ready?` badges on your own draft PRs (read-only draft indicators on others'), and a two-step inline `Approve` button on PR rows requiring approval (hidden when CI checks are failing). A "blocked" filter catches PRs where checks are failing or a review requested changes. The scope filter works here too. Reviewer avatars stack for multiple reviewers. +Open PRs with CI status dots (green for passing, pulsing yellow for checks in progress, solid amber for checks pending human action, ringed yellow for checks pending a condition, red for failing) and inline `Pending action` / `Pending condition` badges, review decision badges, a read-only `auto` badge when GitHub auto-merge is enabled (with merge method tooltip), size badges (XS–XL by lines changed), clickable two-step `Draft` → `Ready?` badges on your own draft PRs (read-only draft indicators on others'), and a two-step inline `Approve` button on PR rows requiring approval (hidden when CI checks are failing). A "blocked" filter catches PRs where checks are failing or a review requested changes. The scope filter works here too. Reviewer avatars stack for multiple reviewers. ### Actions -Workflow runs grouped by repo and workflow name, with duration, triggering actor, and conclusion badges. Accordion collapse per group. A toggle hides runs triggered by PRs so you can focus on branch/schedule runs. +Workflow runs grouped by repo and workflow name, with duration, triggering actor, and status/conclusion indicators (including `Pending action` for deployment approval gates and `Pending condition` for queued/pending runs). Accordion collapse per group. A toggle hides runs triggered by PRs so you can focus on branch/schedule runs. ### Personal Summary Strip @@ -57,7 +57,7 @@ When you sign in, the app searches for repos you've interacted with that aren't ### Hot Polling -A second, faster poll loop (default 30s, configurable 10–120s) targets only in-flight items — PRs with pending CI checks and actively running workflow runs. These are updated via minimal GraphQL `nodes()` queries and individual REST calls rather than full re-fetches, keeping API usage low during active development. +A second, faster poll loop (default 30s, configurable 10–120s) targets only actively running items — PRs with actively running (`Checks running`) or stale CI checks (plus a 60s window after inline `Approve` or `Ready?` actions) and in-progress/queued workflow runs, while skipping checks paused on human action (`Pending action`) or conditions (`Pending condition`). These are updated via minimal GraphQL `nodes()` queries and individual REST calls rather than full re-fetches, keeping API usage low during active development. ### Desktop Notifications @@ -81,7 +81,7 @@ Star counts appear in repo group headers, fetched as part of the standard data r ### Dependencies Tab -An auto-detected built-in tab that appears when dependency bot PRs are found in your tracked repos. Dependency PRs are identified via a multi-layer bot detection pipeline (PR author login, branch name prefix, configurable label). Items are grouped by status — **Mergeable** (CI passing or no CI configured, not a draft, and no changes requested), **Pending Rebase**, **Needs Action** (with explicit reason badges for failing/running checks, merge conflicts, drafts, or requested changes), and **Stale** (not updated in over 14 days) — rather than by repo. Abandoned dependency pills link directly to the Renovate Dashboard issue for bulk resolution. Configurable via **Settings > Dependencies**. +An auto-detected built-in tab that appears when dependency bot PRs are found in your tracked repos. Dependency PRs are identified via a multi-layer bot detection pipeline (PR author login, branch name prefix, configurable label). Items are grouped by status — **Mergeable** (CI passing or no CI configured, not a draft, and no changes requested), **Pending Rebase**, **Needs Action** (with explicit reason badges for `Failing checks`, `Merge conflict`, `Checks running`, `Pending action`, `Pending condition`, `Checking status`, `Draft`, or `Changes`), and **Stale** (not updated in over 14 days) — rather than by repo. Abandoned dependency pills link directly to the Renovate Dashboard issue for bulk resolution. Configurable via **Settings > Dependencies**. ### Custom Tabs diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index d02b242c..d2a153e1 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -208,11 +208,13 @@ A small colored dot shows the aggregate CI status for the PR's latest commit: |-------|---------| | Green (solid) | All checks passed | | Yellow (pulsing) | Checks in progress | +| Amber (solid) | Checks pending human action (e.g., `/ok-to-test` or environment approval) | +| Yellow (ringed/muted) | Checks pending condition (queued or waiting on branch rules/merge queue) | | Red (solid) | Checks failing | | Yellow/faded (solid) | Checks blocked by merge conflict | | Gray (solid) | No checks | -The dot links to the PR's checks page on GitHub. +The dot links to the PR's checks page on GitHub. When checks are paused on human action or a pending condition, the PR row also displays an inline `Pending action` or `Pending condition` badge (in both comfortable and compact density across the Pull Requests and Tracked tabs). #### Review Badges @@ -269,7 +271,7 @@ Shows your involvement in the PR: **Author**, **Reviewer**, **Assignee**, or **I | Role | All / Author / Reviewer / Assignee | All | | Review | All / Approved / Changes / Needs review / Mergeable | All | | Status | All / Draft / Ready | All | -| Checks | All / Passing / Failing / Pending / Conflict / Blocked / No CI | All | +| Checks | All / Passing / Failing / Running / Pending action / Pending condition / Conflict / Blocked / No CI | All | | Size | All / XS / S / M / L / XL | All | | User | All / (tracked user logins) | All (shown when tracked users are configured) | @@ -279,7 +281,7 @@ Shows your involvement in the PR: **Author**, **Reviewer**, **Assignee**, or **I ### Pull Requests Sorting -Sort by: Repo, Title, Author, Checks, Review, Size, Created, Updated (default: Updated, descending). +Sort by: Repo, Title, Author, Checks (failing → conflict → pending action → running → pending condition → passing → none), Review, Size, Created, Updated (default: Updated, descending). --- @@ -289,7 +291,7 @@ The Actions tab is enabled by default. You can disable it in **Settings > GitHub ### Workflow Grouping -Workflow runs are grouped first by repository, then by workflow name. Each workflow group shows its most recent runs up to the configured limit (default: 3 runs per workflow, up to 5 workflows per repo). +Workflow runs are grouped first by repository, then by workflow name. Each workflow group shows its most recent runs up to the configured limit (default: 3 runs per workflow, up to 5 workflows per repo). Collapsed repository headers and workflow summary cards display counts and status accents for passing, failing, running, `pending action` (such as deployment or environment approval gates), and `pending condition` (queued, pending, or requested runs), and individual run rows show matching status icons and duration labels (`running...`, `pending action`, `pending condition`). ### Show PR Runs @@ -299,7 +301,7 @@ By default, runs triggered by pull request events are hidden to reduce noise. To | Filter | Options | Default | |--------|---------|---------| -| Conclusion | All / Success / Failure / Cancelled / Running / Other | All | +| Result (Conclusion) | All / Success / Failure / Cancelled / Running / Pending action / Pending condition / Other | All | | Event | All / Push / Pull request / Schedule / Workflow dispatch / Other | All | --- @@ -328,7 +330,7 @@ Unlike the Pull Requests tab (which groups by repo), the Dependencies tab groups |-------|----------| | **Mergeable** | CI passing (or no CI configured), not a draft, and no changes requested — ready to review or merge | | **Pending Rebase** | PR has the configured rebase label — waiting for the bot to rebase | -| **Needs Action** | Failing or running CI checks, check status still being fetched by the app, merge conflicts, draft PR, or changes requested — rows display explicit reason badges (`Failing checks`, `Merge conflict`, `Checks running`, `Checking status`, `Draft`, `Changes`) | +| **Needs Action** | Failing, running, pending-action, or pending-condition CI checks, check status still being fetched by the app, merge conflicts, draft PR, or changes requested — rows display explicit reason badges (`Failing checks`, `Merge conflict`, `Checks running`, `Pending action`, `Pending condition`, `Checking status`, `Draft`, `Changes`) | | **Stale** | PR has not been updated in over 14 days — may need a rebase or manual review | Reason badges reflect each PR's current state, so the same badges also appear on rows in the **Pending Rebase** and **Stale** groups. On initial load, and during a refresh in which no PR's details have loaded yet, `Checking status` is withheld on dependency PRs without details, to avoid a flash of the badge on every row. @@ -405,7 +407,7 @@ Upstream repos are included in Issues and Pull Requests fetches but excluded fro The dashboard polls GitHub at a configurable interval (default: **5 minutes**). Each full refresh fetches all issues, PRs, and workflow runs. You can trigger a manual refresh by clicking the refresh button in the toolbar. -PR enrichment queries (reviewer lists, CI check rollups, merge conflict status, and line-change counts) run in 25-PR batches. If GitHub's GraphQL API is slow, returns a server error, or the request fails outright (`Failed to fetch`, which is how GitHub gateway timeouts appear in the browser), a failing batch is split in half repeatedly until batches are 6 PRs or fewer, then a lightweight fallback query fills in everything except CI check and merge-conflict status. Once a second batch in the same refresh fails before any PR's full details have loaded, later failing batches skip splitting and go straight to the fallback. Dependency PR and Renovate Dashboard body fetches also run in 25-node batches and split failing batches the same way (except after the 45-second safety timeout); any items still failing are retried once a 5-minute cooldown passes. Settled PR enrichments — where CI checks are not pending and the PR's `updatedAt` timestamp is unchanged — are cached in memory for your refresh interval plus one minute (at least 3 minutes), so a settled PR skips the enrichment query on the next automatic poll and is re-fetched on the one after (the cache is also invalidated automatically when repository events or PR actions occur). Changes that don't update a PR's `updatedAt` — such as a CI re-run, or a merge conflict caused by new commits on its base branch — aren't picked up while the cached entry is valid, so such a PR can show its previous check or merge status (including the **Approve** button) for up to about two refresh intervals. Clicking the manual **Refresh** button always bypasses this cache. +PR enrichment queries (reviewer lists, CI check rollups, merge conflict status, and line-change counts) run in 25-PR batches. If GitHub's GraphQL API is slow, returns a server error, or the request fails outright (`Failed to fetch`, which is how GitHub gateway timeouts appear in the browser), a failing batch is split in half repeatedly until batches are 6 PRs or fewer, then a lightweight fallback query fills in everything except CI check and merge-conflict status. Once a second batch in the same refresh fails before any PR's full details have loaded, later failing batches skip splitting and go straight to the fallback. Dependency PR and Renovate Dashboard body fetches also run in 25-node batches and split failing batches the same way (except after the 45-second safety timeout); any items still failing are retried once a 5-minute cooldown passes. Settled PR enrichments — where CI checks are in a terminal state (not `running`, `waiting`, or `pending`) and the PR's `updatedAt` timestamp is unchanged — are cached in memory for your refresh interval plus one minute (at least 3 minutes), so a settled PR skips the enrichment query on the next automatic poll and is re-fetched on the one after (the cache is also invalidated automatically when repository events or PR actions occur). Changes that don't update a PR's `updatedAt` — such as a CI re-run, or a merge conflict caused by new commits on its base branch — aren't picked up while the cached entry is valid, so such a PR can show its previous check or merge status (including the **Approve** button) for up to about two refresh intervals. Clicking the manual **Refresh** button always bypasses this cache. A subtle amber **Partial sync** indicator appears next to the Refresh button, while cached PR details are retained, when PR enrichment fails during a refresh and either leaves any PR without details or with unverified CI check status, or is rejected outright by GitHub (for example, a permissions error); otherwise it appears once enrichment failures persist across 3 consecutive polls. PRs whose CI check status couldn't be verified — filled in by the lightweight fallback query, changed (new `headSha` or `updatedAt`) during a degraded backfill, or forks whose CI lookup failed — have their check status cleared (`checkStatusStale`) rather than carried forward: they show `Checking status`, hide **Approve**, and are resolved automatically by the next hot-poll cycle or successful refresh. The indicator clears on the next full refresh whose PR enrichment succeeds, unless a search failure in that refresh leaves a PR still in this unverified state. @@ -421,8 +423,8 @@ Each full refresh also checks GitHub's own status page for outages affecting Act A second, faster poll loop runs alongside the full refresh specifically for in-flight items. It targets: -- **PRs with pending or stale (`checkStatusStale`) CI checks** — re-checks status until checks resolve (including PRs whose commit changed during a degraded backfill). For PRs from forks whose CI runs only in the fork, the status is read from the fork's head commit, the same source the full refresh uses. -- **In-progress workflow runs** — re-checks until the run completes +- **PRs with actively running (`running`) or stale (`checkStatusStale`) CI checks** — re-checks status until checks leave the running state (plus a 60-second window after an inline `Approve` or `Ready?` action to catch newly triggered workflows; PRs paused in `waiting` or `pending` are not hot-polled). For PRs from forks whose CI runs only in the fork, the status is read from the fork's head commit, the same source the full refresh uses. +- **In-progress or queued workflow runs** — re-checks until the run completes or pauses on an environment approval gate (`waiting`) Default interval: **30 seconds** (configurable from 10 to 120 seconds in Settings). diff --git a/src/app/components/dashboard/ActionsTab.tsx b/src/app/components/dashboard/ActionsTab.tsx index cc8da239..761908e2 100644 --- a/src/app/components/dashboard/ActionsTab.tsx +++ b/src/app/components/dashboard/ActionsTab.tsx @@ -3,7 +3,7 @@ import { createStore } from "solid-js/store"; import type { WorkflowRun } from "../../services/api"; import { viewState, setViewState, ignoreItem, unignoreItem, toggleExpandedRepo, setAllExpanded, isRepoExpanded, pruneExpandedRepos, pruneLockedRepos, ActionsFiltersSchema } from "../../stores/view"; import { createTabFilterHandlers, mergeActiveFilters } from "../../lib/tabFilters"; -import { isRunVisible } from "../../lib/filters"; +import { isRunVisible, isWorkflowRunPending, isWorkflowRunWaiting } from "../../lib/filters"; import WorkflowSummaryCard from "./WorkflowSummaryCard"; import IgnoreBadge from "./IgnoreBadge"; import SkeletonRows from "../shared/SkeletonRows"; @@ -93,10 +93,12 @@ function sortWorkflowsByStatus(workflows: WorkflowGroup[]): WorkflowGroup[] { return workflows.toSorted((a, b) => { const priorityOf = (wf: WorkflowGroup): number => { const latest = wf.runs[0]; - if (!latest) return 2; + if (!latest) return 4; + if (isWorkflowRunWaiting(latest)) return 1; + if (latest.status === "in_progress") return 2; + if (isWorkflowRunPending(latest)) return 3; if (latest.conclusion === "failure") return 0; - if (latest.status === "in_progress") return 1; - return 2; + return 4; }; return priorityOf(a) - priorityOf(b); }); @@ -168,6 +170,10 @@ export default function ActionsTab(props: ActionsTabProps) { if (conclusionFilter !== "all") { if (conclusionFilter === "running") { if (run.status !== "in_progress") return false; + } else if (conclusionFilter === "waiting") { + if (!isWorkflowRunWaiting(run)) return false; + } else if (conclusionFilter === "pending") { + if (!isWorkflowRunPending(run)) return false; } else if (conclusionFilter === "other") { if (run.conclusion === null || (KNOWN_CONCLUSIONS as readonly string[]).includes(run.conclusion)) return false; } else { @@ -268,13 +274,18 @@ export default function ActionsTab(props: ActionsTabProps) { let passed = 0; let failed = 0; let running = 0; + let waiting = 0; + let pending = 0; for (const wf of wfs) { const latest = wf.runs[0]; - if (latest?.conclusion === "success") passed++; - else if (latest?.conclusion === "failure") failed++; - else if (latest?.status === "in_progress") running++; + if (!latest) continue; + if (latest.status === "in_progress") running++; + else if (isWorkflowRunWaiting(latest)) waiting++; + else if (isWorkflowRunPending(latest)) pending++; + else if (latest.conclusion === "success") passed++; + else if (latest.conclusion === "failure") failed++; } - return { total, passed, failed, running }; + return { total, passed, failed, running, waiting, pending }; }); return ( @@ -299,23 +310,35 @@ export default function ActionsTab(props: ActionsTabProps) { collapsedSummary={ {workflowCounts().total} workflow{workflowCounts().total !== 1 ? "s" : ""} - 0 || workflowCounts().failed > 0 || workflowCounts().running > 0}> + 0 || workflowCounts().failed > 0 || workflowCounts().running > 0 || workflowCounts().waiting > 0 || workflowCounts().pending > 0}> {": "} 0}> {workflowCounts().passed} passed - 0 && (workflowCounts().failed > 0 || workflowCounts().running > 0)}> + 0 && (workflowCounts().failed > 0 || workflowCounts().running > 0 || workflowCounts().waiting > 0 || workflowCounts().pending > 0)}> {", "} 0}> {workflowCounts().failed} failed - 0 && workflowCounts().running > 0}> + 0 && (workflowCounts().running > 0 || workflowCounts().waiting > 0 || workflowCounts().pending > 0)}> {", "} 0}> {workflowCounts().running} running + 0 && (workflowCounts().waiting > 0 || workflowCounts().pending > 0)}> + {", "} + + 0}> + {workflowCounts().waiting} pending action + + 0 && workflowCounts().pending > 0}> + {", "} + + 0}> + {workflowCounts().pending} pending condition + } diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index d7634de6..2779f2c8 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -45,7 +45,7 @@ import { canApprovePR, canMarkPRReady, formatCount, prSizeCategory, rateLimitCss import { setsEqual } from "../../lib/collections"; import { withScrollLock } from "../../lib/scroll"; import { Tooltip } from "../shared/Tooltip"; -import { isIssueVisible, isPrVisible, isRunVisible } from "../../lib/filters"; +import { isIssueVisible, isPrVisible, isRunVisible, isWorkflowRunPending, isWorkflowRunWaiting } from "../../lib/filters"; import { isUserInvolved } from "../../lib/grouping"; import { KNOWN_CONCLUSIONS, KNOWN_EVENTS } from "../shared/filterTypes"; import CustomTabModal from "../shared/CustomTabModal"; @@ -1380,6 +1380,8 @@ export default function DashboardPage() { if (!isRunVisible(w, { ignoredIds: ignoredRuns, showPrRuns: viewState.showPrRuns, globalFilter: null })) return false; if (f.conclusion !== "all") { if (f.conclusion === "running") { if (w.status !== "in_progress") return false; } + else if (f.conclusion === "waiting") { if (!isWorkflowRunWaiting(w)) return false; } + else if (f.conclusion === "pending") { if (!isWorkflowRunPending(w)) return false; } else if (f.conclusion === "other") { if (w.conclusion === null || (KNOWN_CONCLUSIONS as readonly string[]).includes(w.conclusion)) return false; } else if (w.conclusion !== f.conclusion) return false; } diff --git a/src/app/components/dashboard/WorkflowRunRow.tsx b/src/app/components/dashboard/WorkflowRunRow.tsx index b98858a3..bee7d574 100644 --- a/src/app/components/dashboard/WorkflowRunRow.tsx +++ b/src/app/components/dashboard/WorkflowRunRow.tsx @@ -1,5 +1,6 @@ import { createMemo, Show } from "solid-js"; import type { WorkflowRun } from "../../services/api"; +import { isWorkflowRunPending, isWorkflowRunWaiting } from "../../lib/filters"; import { isSafeGitHubUrl } from "../../lib/url"; import { relativeTime, formatDuration } from "../../lib/format"; import { Tooltip } from "../shared/Tooltip"; @@ -101,8 +102,28 @@ function StatusIcon(props: { status: string; conclusion: string | null }) { ); } - // queued or unknown → gray clock - const label = props.status === "queued" ? "Queued" : "Unknown status"; + // waiting / action_required → amber pause/action icon + if (isWorkflowRunWaiting(props)) { + return ( + + + + ); + } + + // queued / pending / requested or unknown → gray clock + const label = isWorkflowRunPending(props) ? "Pending condition" : "Unknown status"; return ( { - const { failure, running, success } = counts(); + const { failure, waiting, running, pending, success } = counts(); if (failure > 0) return "red"; + if (waiting > 0) return "amber"; if (running > 0) return "yellow"; + if (pending > 0) return "pending"; if (success > 0 && success === props.runs.length) return "green"; return "gray"; }); @@ -40,7 +47,9 @@ export default function WorkflowSummaryCard(props: WorkflowSummaryCardProps) { const borderLeftClass = createMemo(() => { const color = accentColor(); if (color === "red") return "border-l-4 border-l-error"; + if (color === "amber") return "border-l-4 border-l-amber-500"; if (color === "yellow") return "border-l-4 border-l-warning"; + if (color === "pending") return "border-l-4 border-l-warning/50"; if (color === "green") return "border-l-4 border-l-success"; return "border-l-4 border-l-base-300"; }); @@ -101,6 +110,20 @@ export default function WorkflowSummaryCard(props: WorkflowSummaryCardProps) { + 0}> + + + {counts().waiting} + + + + 0}> + + + {counts().pending} + + + diff --git a/src/app/lib/filters.ts b/src/app/lib/filters.ts index 789ca73a..5e46d9cd 100644 --- a/src/app/lib/filters.ts +++ b/src/app/lib/filters.ts @@ -39,3 +39,15 @@ export function isRunVisible(run: WorkflowRun, opts: ItemFilterOpts): boolean { } return true; } + +export function isWorkflowRunWaiting(run: Pick): boolean { + return ( + run.status === "waiting" || + run.status === "action_required" || + (run.status === "completed" && run.conclusion === "action_required") + ); +} + +export function isWorkflowRunPending(run: Pick): boolean { + return run.status === "queued" || run.status === "pending" || run.status === "requested"; +} diff --git a/tests/components/ActionsTab.test.tsx b/tests/components/ActionsTab.test.tsx index 669d992e..6bfdd86e 100644 --- a/tests/components/ActionsTab.test.tsx +++ b/tests/components/ActionsTab.test.tsx @@ -491,4 +491,63 @@ describe("ActionsTab", () => { expect(row.classList.contains("animate-shimmer")).toBe(false); } }); + + it("sorts workflows by status priority: failure < waiting < in_progress < queued/pending < success", async () => { + const user = userEvent.setup(); + const runs = [ + makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "WF-Success", status: "completed", conclusion: "success" }), + makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "WF-Pending", status: "queued", conclusion: null }), + makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-Running", status: "in_progress", conclusion: null }), + makeWorkflowRun({ id: 4, repoFullName: "owner/repo", workflowId: 4, name: "WF-Waiting", status: "waiting", conclusion: null }), + makeWorkflowRun({ id: 5, repoFullName: "owner/repo", workflowId: 5, name: "WF-Failure", status: "completed", conclusion: "failure" }), + ]; + render(() => ); + + await user.click(screen.getByText("owner/repo")); + + const cards = screen.getAllByText(/^WF-/); + expect(cards.map((el) => el.textContent)).toEqual([ + "WF-Failure", + "WF-Waiting", + "WF-Running", + "WF-Pending", + "WF-Success", + ]); + }); + + it("displays pending action and pending condition counts in repo-group collapsed summary", () => { + const runs = [ + makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "Deploy", status: "waiting", conclusion: null }), + makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "E2E", status: "queued", conclusion: null }), + makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "Build", status: "in_progress", conclusion: null }), + ]; + render(() => ); + + const summaryEl = screen.getByText(/3 workflows/); + expect(summaryEl.textContent).toContain("1 running"); + expect(summaryEl.textContent).toContain("1 pending action"); + expect(summaryEl.textContent).toContain("1 pending condition"); + }); + + it("filters workflow runs by conclusion='waiting' and conclusion='pending'", async () => { + const user = userEvent.setup(); + const runs = [ + makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "WF-Wait", status: "waiting", conclusion: null, displayTitle: "waiting-run" }), + makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "WF-Pend", status: "queued", conclusion: null, displayTitle: "pending-run" }), + makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-Pass", status: "completed", conclusion: "success", displayTitle: "passing-run" }), + ]; + + viewStore.setTabFilter("actions", "conclusion", "waiting"); + render(() => ); + + await user.click(screen.getByText("owner/repo")); + expect(screen.getAllByText("WF-Wait").length).toBeGreaterThanOrEqual(1); + expect(screen.queryByText("WF-Pend")).toBeNull(); + expect(screen.queryByText("WF-Pass")).toBeNull(); + + viewStore.setTabFilter("actions", "conclusion", "pending"); + expect(screen.queryByText("WF-Wait")).toBeNull(); + expect(screen.getAllByText("WF-Pend").length).toBeGreaterThanOrEqual(1); + expect(screen.queryByText("WF-Pass")).toBeNull(); + }); }); diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index c81d023d..de238b7b 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -1750,6 +1750,50 @@ describe("DashboardPage — tabCounts applies filterPreset", () => { }); }); + it("conclusion:waiting and conclusion:pending presets count matching workflow runs accurately on actions custom tabs", async () => { + configStore.addCustomTab({ + id: "waitingruns", + name: "Waiting Runs", + baseType: "actions", + orgScope: ["owner"], + repoScope: [], + filterPreset: { conclusion: "waiting" }, + exclusive: false, + }); + configStore.addCustomTab({ + id: "pendingruns", + name: "Pending Runs", + baseType: "actions", + orgScope: ["owner"], + repoScope: [], + filterPreset: { conclusion: "pending" }, + exclusive: false, + }); + + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [], + workflowRuns: [ + makeWorkflowRun({ id: 20, status: "waiting", conclusion: null, isPrRun: false }), + makeWorkflowRun({ id: 21, status: "completed", conclusion: "action_required", isPrRun: false }), + makeWorkflowRun({ id: 22, status: "queued", conclusion: null, isPrRun: false }), + makeWorkflowRun({ id: 23, status: "pending", conclusion: null, isPrRun: false }), + makeWorkflowRun({ id: 24, status: "requested", conclusion: null, isPrRun: false }), + makeWorkflowRun({ id: 25, status: "in_progress", conclusion: null, isPrRun: false }), + makeWorkflowRun({ id: 26, status: "completed", conclusion: "success", isPrRun: false }), + ], + errors: [], + }); + + render(() => ); + await waitFor(() => { + const waitingTab = screen.getByRole("tab", { name: /Waiting Runs/ }); + expect(waitingTab.textContent?.replace(/\D+/g, "")).toBe("2"); + const pendingTab = screen.getByRole("tab", { name: /Pending Runs/ }); + expect(pendingTab.textContent?.replace(/\D+/g, "")).toBe("3"); + }); + }); + describe("Jira auth guard", () => { it("does not write Jira issues when auth becomes invalid during fetch", async () => { let authenticated = true; diff --git a/tests/components/WorkflowRunRow.test.tsx b/tests/components/WorkflowRunRow.test.tsx index a88f1872..f142c8d4 100644 --- a/tests/components/WorkflowRunRow.test.tsx +++ b/tests/components/WorkflowRunRow.test.tsx @@ -107,12 +107,56 @@ describe("WorkflowRunRow", () => { screen.getByLabelText("In progress"); }); - it("renders status indicator for queued status", () => { + it("renders status indicator and duration for queued status as Pending condition", () => { const run = makeWorkflowRun({ status: "queued", conclusion: null }); render(() => ( {}} /> )); - screen.getByLabelText("Queued"); + screen.getByLabelText("Pending condition"); + screen.getByText("pending condition"); + }); + + it("renders status indicator and duration for waiting and action_required statuses as Pending action", () => { + const waitingRun = makeWorkflowRun({ id: 201, status: "waiting", conclusion: null }); + const { unmount } = render(() => ( + {}} /> + )); + const icon = screen.getByLabelText("Pending action"); + expect(icon.getAttribute("class")).toContain("text-amber-500"); + screen.getByText("pending action"); + unmount(); + + const actionReqRun = makeWorkflowRun({ id: 202, status: "action_required", conclusion: null }); + const { unmount: unmount2 } = render(() => ( + {}} /> + )); + screen.getByLabelText("Pending action"); + screen.getByText("pending action"); + unmount2(); + + const completedActionReqRun = makeWorkflowRun({ id: 203, status: "completed", conclusion: "action_required" }); + render(() => ( + {}} /> + )); + screen.getByLabelText("Pending action"); + screen.getByText("pending action"); + }); + + it("renders status indicator and duration for pending and requested statuses as Pending condition", () => { + const pendingRun = makeWorkflowRun({ id: 204, status: "pending", conclusion: null }); + const { unmount } = render(() => ( + {}} /> + )); + screen.getByLabelText("Pending condition"); + screen.getByText("pending condition"); + unmount(); + + const requestedRun = makeWorkflowRun({ id: 205, status: "requested", conclusion: null }); + render(() => ( + {}} /> + )); + screen.getByLabelText("Pending condition"); + screen.getByText("pending condition"); }); it("calls onIgnore when ignore button clicked", async () => { diff --git a/tests/components/WorkflowSummaryCard.test.tsx b/tests/components/WorkflowSummaryCard.test.tsx index 7345cb63..6fd0098d 100644 --- a/tests/components/WorkflowSummaryCard.test.tsx +++ b/tests/components/WorkflowSummaryCard.test.tsx @@ -176,6 +176,45 @@ describe("WorkflowSummaryCard", () => { expect(card.className).toContain("border-l-warning"); }); + it("shows waiting ('1 pending action', text-amber-500, border-l-amber-500) and pending ('1 pending condition', text-base-content/60, border-l-warning/50) counts and border accents", () => { + const waitingRuns = [ + makeWorkflowRun({ id: 1, status: "waiting", conclusion: null }), + ]; + const { container: waitingContainer, unmount } = render(() => ( + {}} + onIgnoreRun={() => {}} + /> + )); + const waitingCard = waitingContainer.firstElementChild as HTMLElement; + expect(waitingCard.className).toContain("border-l-amber-500"); + const waitingCountEl = screen.getByLabelText("1 pending action"); + expect(waitingCountEl.textContent).toBe("1"); + expect(waitingCountEl.className).toContain("text-amber-500"); + unmount(); + + const pendingRuns = [ + makeWorkflowRun({ id: 2, status: "queued", conclusion: null }), + ]; + const { container: pendingContainer } = render(() => ( + {}} + onIgnoreRun={() => {}} + /> + )); + const pendingCard = pendingContainer.firstElementChild as HTMLElement; + expect(pendingCard.className).toContain("border-l-warning/50"); + const pendingCountEl = screen.getByLabelText("1 pending condition"); + expect(pendingCountEl.textContent).toBe("1"); + expect(pendingCountEl.className).toContain("text-base-content/60"); + }); + it("passes isPolling to WorkflowRunRow when hotPollingRunIds contains run ID", () => { const runs = [ makeWorkflowRun({ id: 10, conclusion: null, status: "in_progress" }), From 59095c2bafc247b0c0bdac12f4c25416d4aa7323 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 21:58:55 -0400 Subject: [PATCH 04/10] refactor(actions): simplify workflow status helpers and finalize tests Deduplicate the non-terminal waiting/pending icon branch in WorkflowRunRow and remove a redundant null guard in ActionsTab sortWorkflowsByStatus. Finalize C4 test additions in ActionsTab.test.tsx and DashboardPage.test.tsx. --- tests/components/ActionsTab.test.tsx | 22 ++++++++++++++++------ tests/components/DashboardPage.test.tsx | 3 ++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/components/ActionsTab.test.tsx b/tests/components/ActionsTab.test.tsx index 6bfdd86e..42176548 100644 --- a/tests/components/ActionsTab.test.tsx +++ b/tests/components/ActionsTab.test.tsx @@ -520,21 +520,27 @@ describe("ActionsTab", () => { makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "Deploy", status: "waiting", conclusion: null }), makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "E2E", status: "queued", conclusion: null }), makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "Build", status: "in_progress", conclusion: null }), + makeWorkflowRun({ id: 4, repoFullName: "owner/repo", workflowId: 4, name: "Lint", status: "completed", conclusion: "success" }), + makeWorkflowRun({ id: 5, repoFullName: "owner/repo", workflowId: 5, name: "Sec", status: "completed", conclusion: "failure" }), ]; render(() => ); - const summaryEl = screen.getByText(/3 workflows/); - expect(summaryEl.textContent).toContain("1 running"); - expect(summaryEl.textContent).toContain("1 pending action"); - expect(summaryEl.textContent).toContain("1 pending condition"); + const summaryEl = screen.getByText(/5 workflows/); + expect(summaryEl.textContent).toBe( + "5 workflows: 1 passed, 1 failed, 1 running, 1 pending action, 1 pending condition" + ); + const waitingSpan = screen.getByText("1 pending action"); + expect(waitingSpan.className).toContain("text-amber-500"); }); it("filters workflow runs by conclusion='waiting' and conclusion='pending'", async () => { const user = userEvent.setup(); const runs = [ makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "WF-Wait", status: "waiting", conclusion: null, displayTitle: "waiting-run" }), - makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "WF-Pend", status: "queued", conclusion: null, displayTitle: "pending-run" }), - makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-Pass", status: "completed", conclusion: "success", displayTitle: "passing-run" }), + makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "WF-ActReq", status: "completed", conclusion: "action_required", displayTitle: "actreq-run" }), + makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-Pend", status: "queued", conclusion: null, displayTitle: "pending-run" }), + makeWorkflowRun({ id: 4, repoFullName: "owner/repo", workflowId: 4, name: "WF-Req", status: "requested", conclusion: null, displayTitle: "req-run" }), + makeWorkflowRun({ id: 5, repoFullName: "owner/repo", workflowId: 5, name: "WF-Pass", status: "completed", conclusion: "success", displayTitle: "passing-run" }), ]; viewStore.setTabFilter("actions", "conclusion", "waiting"); @@ -542,12 +548,16 @@ describe("ActionsTab", () => { await user.click(screen.getByText("owner/repo")); expect(screen.getAllByText("WF-Wait").length).toBeGreaterThanOrEqual(1); + expect(screen.getAllByText("WF-ActReq").length).toBeGreaterThanOrEqual(1); expect(screen.queryByText("WF-Pend")).toBeNull(); + expect(screen.queryByText("WF-Req")).toBeNull(); expect(screen.queryByText("WF-Pass")).toBeNull(); viewStore.setTabFilter("actions", "conclusion", "pending"); expect(screen.queryByText("WF-Wait")).toBeNull(); + expect(screen.queryByText("WF-ActReq")).toBeNull(); expect(screen.getAllByText("WF-Pend").length).toBeGreaterThanOrEqual(1); + expect(screen.getAllByText("WF-Req").length).toBeGreaterThanOrEqual(1); expect(screen.queryByText("WF-Pass")).toBeNull(); }); }); diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index de238b7b..d5e246c8 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -1781,6 +1781,7 @@ describe("DashboardPage — tabCounts applies filterPreset", () => { makeWorkflowRun({ id: 24, status: "requested", conclusion: null, isPrRun: false }), makeWorkflowRun({ id: 25, status: "in_progress", conclusion: null, isPrRun: false }), makeWorkflowRun({ id: 26, status: "completed", conclusion: "success", isPrRun: false }), + makeWorkflowRun({ id: 27, status: "action_required", conclusion: null, isPrRun: false }), ], errors: [], }); @@ -1788,7 +1789,7 @@ describe("DashboardPage — tabCounts applies filterPreset", () => { render(() => ); await waitFor(() => { const waitingTab = screen.getByRole("tab", { name: /Waiting Runs/ }); - expect(waitingTab.textContent?.replace(/\D+/g, "")).toBe("2"); + expect(waitingTab.textContent?.replace(/\D+/g, "")).toBe("3"); const pendingTab = screen.getByRole("tab", { name: /Pending Runs/ }); expect(pendingTab.textContent?.replace(/\D+/g, "")).toBe("3"); }); From dfe54029050f53959513d4de7ba3547ec5956827 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 22:01:24 -0400 Subject: [PATCH 05/10] docs(guide): update quick-start and tracked-tab check status docs Update the Quick Start hot-poll tip and Tracked Tab PR row description in docs/USER_GUIDE.md to reflect the 3-way non-terminal check states (running, Pending action, and Pending condition), deduplicate status badge JSX in PullRequestsTab and TrackedTab, and include final C4 ActionsTab/DashboardPage test additions. --- tests/components/ActionsTab.test.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/components/ActionsTab.test.tsx b/tests/components/ActionsTab.test.tsx index 42176548..0ed5a466 100644 --- a/tests/components/ActionsTab.test.tsx +++ b/tests/components/ActionsTab.test.tsx @@ -538,9 +538,11 @@ describe("ActionsTab", () => { const runs = [ makeWorkflowRun({ id: 1, repoFullName: "owner/repo", workflowId: 1, name: "WF-Wait", status: "waiting", conclusion: null, displayTitle: "waiting-run" }), makeWorkflowRun({ id: 2, repoFullName: "owner/repo", workflowId: 2, name: "WF-ActReq", status: "completed", conclusion: "action_required", displayTitle: "actreq-run" }), - makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-Pend", status: "queued", conclusion: null, displayTitle: "pending-run" }), - makeWorkflowRun({ id: 4, repoFullName: "owner/repo", workflowId: 4, name: "WF-Req", status: "requested", conclusion: null, displayTitle: "req-run" }), - makeWorkflowRun({ id: 5, repoFullName: "owner/repo", workflowId: 5, name: "WF-Pass", status: "completed", conclusion: "success", displayTitle: "passing-run" }), + makeWorkflowRun({ id: 3, repoFullName: "owner/repo", workflowId: 3, name: "WF-ActReqStatus", status: "action_required", conclusion: null, displayTitle: "actreq-status-run" }), + makeWorkflowRun({ id: 4, repoFullName: "owner/repo", workflowId: 4, name: "WF-Queued", status: "queued", conclusion: null, displayTitle: "queued-run" }), + makeWorkflowRun({ id: 5, repoFullName: "owner/repo", workflowId: 5, name: "WF-Pend", status: "pending", conclusion: null, displayTitle: "pending-run" }), + makeWorkflowRun({ id: 6, repoFullName: "owner/repo", workflowId: 6, name: "WF-Req", status: "requested", conclusion: null, displayTitle: "req-run" }), + makeWorkflowRun({ id: 7, repoFullName: "owner/repo", workflowId: 7, name: "WF-Pass", status: "completed", conclusion: "success", displayTitle: "passing-run" }), ]; viewStore.setTabFilter("actions", "conclusion", "waiting"); @@ -549,6 +551,8 @@ describe("ActionsTab", () => { await user.click(screen.getByText("owner/repo")); expect(screen.getAllByText("WF-Wait").length).toBeGreaterThanOrEqual(1); expect(screen.getAllByText("WF-ActReq").length).toBeGreaterThanOrEqual(1); + expect(screen.getAllByText("WF-ActReqStatus").length).toBeGreaterThanOrEqual(1); + expect(screen.queryByText("WF-Queued")).toBeNull(); expect(screen.queryByText("WF-Pend")).toBeNull(); expect(screen.queryByText("WF-Req")).toBeNull(); expect(screen.queryByText("WF-Pass")).toBeNull(); @@ -556,6 +560,8 @@ describe("ActionsTab", () => { viewStore.setTabFilter("actions", "conclusion", "pending"); expect(screen.queryByText("WF-Wait")).toBeNull(); expect(screen.queryByText("WF-ActReq")).toBeNull(); + expect(screen.queryByText("WF-ActReqStatus")).toBeNull(); + expect(screen.getAllByText("WF-Queued").length).toBeGreaterThanOrEqual(1); expect(screen.getAllByText("WF-Pend").length).toBeGreaterThanOrEqual(1); expect(screen.getAllByText("WF-Req").length).toBeGreaterThanOrEqual(1); expect(screen.queryByText("WF-Pass")).toBeNull(); From 906ab86131be25383862313b9fb06ca3691db5e0 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 22:07:50 -0400 Subject: [PATCH 06/10] fix(actions): exclude non-terminal runs from terminal conclusion filters --- src/app/components/dashboard/ActionsTab.tsx | 19 +++++++++++++++++-- .../components/dashboard/DashboardPage.tsx | 19 +++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/app/components/dashboard/ActionsTab.tsx b/src/app/components/dashboard/ActionsTab.tsx index 761908e2..d7472ec3 100644 --- a/src/app/components/dashboard/ActionsTab.tsx +++ b/src/app/components/dashboard/ActionsTab.tsx @@ -175,9 +175,24 @@ export default function ActionsTab(props: ActionsTabProps) { } else if (conclusionFilter === "pending") { if (!isWorkflowRunPending(run)) return false; } else if (conclusionFilter === "other") { - if (run.conclusion === null || (KNOWN_CONCLUSIONS as readonly string[]).includes(run.conclusion)) return false; + if ( + run.status === "in_progress" || + isWorkflowRunWaiting(run) || + isWorkflowRunPending(run) || + run.conclusion === null || + (KNOWN_CONCLUSIONS as readonly string[]).includes(run.conclusion) + ) { + return false; + } } else { - if (run.conclusion !== conclusionFilter) return false; + if ( + run.status === "in_progress" || + isWorkflowRunWaiting(run) || + isWorkflowRunPending(run) || + run.conclusion !== conclusionFilter + ) { + return false; + } } } diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 2779f2c8..264a0973 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -1382,8 +1382,23 @@ export default function DashboardPage() { if (f.conclusion === "running") { if (w.status !== "in_progress") return false; } else if (f.conclusion === "waiting") { if (!isWorkflowRunWaiting(w)) return false; } else if (f.conclusion === "pending") { if (!isWorkflowRunPending(w)) return false; } - else if (f.conclusion === "other") { if (w.conclusion === null || (KNOWN_CONCLUSIONS as readonly string[]).includes(w.conclusion)) return false; } - else if (w.conclusion !== f.conclusion) return false; + else if (f.conclusion === "other") { + if ( + w.status === "in_progress" || + isWorkflowRunWaiting(w) || + isWorkflowRunPending(w) || + w.conclusion === null || + (KNOWN_CONCLUSIONS as readonly string[]).includes(w.conclusion) + ) { + return false; + } + } + else if ( + w.status === "in_progress" || + isWorkflowRunWaiting(w) || + isWorkflowRunPending(w) || + w.conclusion !== f.conclusion + ) return false; } if (f.event !== "all") { if (f.event === "other") { if ((KNOWN_EVENTS as readonly string[]).includes(w.event)) return false; } From ada319cbe3f3102175abb094e7d5aa675ae5b5e0 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 22:08:28 -0400 Subject: [PATCH 07/10] test(actions): include final workflow filter and custom-tab assertions --- tests/components/WorkflowRunRow.test.tsx | 21 ++++++++++++--- tests/components/WorkflowSummaryCard.test.tsx | 26 ++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/tests/components/WorkflowRunRow.test.tsx b/tests/components/WorkflowRunRow.test.tsx index f142c8d4..e03722d0 100644 --- a/tests/components/WorkflowRunRow.test.tsx +++ b/tests/components/WorkflowRunRow.test.tsx @@ -134,12 +134,19 @@ describe("WorkflowRunRow", () => { screen.getByText("pending action"); unmount2(); - const completedActionReqRun = makeWorkflowRun({ id: 203, status: "completed", conclusion: "action_required" }); - render(() => ( + const completedActionReqRun = makeWorkflowRun({ + id: 203, + status: "completed", + conclusion: "action_required", + runStartedAt: "2026-03-30T10:00:00Z", + completedAt: "2026-03-30T10:05:00Z", + }); + const { unmount: unmount3 } = render(() => ( {}} /> )); screen.getByLabelText("Pending action"); screen.getByText("pending action"); + unmount3(); }); it("renders status indicator and duration for pending and requested statuses as Pending condition", () => { @@ -152,11 +159,19 @@ describe("WorkflowRunRow", () => { unmount(); const requestedRun = makeWorkflowRun({ id: 205, status: "requested", conclusion: null }); - render(() => ( + const { unmount: unmount2 } = render(() => ( {}} /> )); screen.getByLabelText("Pending condition"); screen.getByText("pending condition"); + unmount2(); + + const unknownRun = makeWorkflowRun({ id: 206, status: "unknown", conclusion: null }); + render(() => ( + {}} /> + )); + screen.getByLabelText("Unknown status"); + screen.getByText("--"); }); it("calls onIgnore when ignore button clicked", async () => { diff --git a/tests/components/WorkflowSummaryCard.test.tsx b/tests/components/WorkflowSummaryCard.test.tsx index 6fd0098d..0b68f2a8 100644 --- a/tests/components/WorkflowSummaryCard.test.tsx +++ b/tests/components/WorkflowSummaryCard.test.tsx @@ -199,7 +199,7 @@ describe("WorkflowSummaryCard", () => { const pendingRuns = [ makeWorkflowRun({ id: 2, status: "queued", conclusion: null }), ]; - const { container: pendingContainer } = render(() => ( + const { container: pendingContainer, unmount: unmountPending } = render(() => ( { const pendingCountEl = screen.getByLabelText("1 pending condition"); expect(pendingCountEl.textContent).toBe("1"); expect(pendingCountEl.className).toContain("text-base-content/60"); + unmountPending(); + + // Mixed waiting + running + pending + success prioritizes amber border accent over yellow/pending/green + const mixedRuns = [ + makeWorkflowRun({ id: 3, status: "completed", conclusion: "action_required" }), + makeWorkflowRun({ id: 4, status: "in_progress", conclusion: null }), + makeWorkflowRun({ id: 5, status: "pending", conclusion: null }), + makeWorkflowRun({ id: 6, status: "completed", conclusion: "success" }), + ]; + const { container: mixedContainer } = render(() => ( + {}} + onIgnoreRun={() => {}} + /> + )); + const mixedCard = mixedContainer.firstElementChild as HTMLElement; + expect(mixedCard.className).toContain("border-l-amber-500"); + expect(screen.getByLabelText("1 pending action").textContent).toBe("1"); + expect(screen.getByLabelText("1 running").textContent).toBe("1"); + expect(screen.getByLabelText("1 pending condition").textContent).toBe("1"); + expect(screen.getByLabelText("1 successful").textContent).toBe("1"); }); it("passes isPolling to WorkflowRunRow when hotPollingRunIds contains run ID", () => { From 84497722abd014fadbb948b1698af4339fec8495 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 30 Oct 2026 22:17:13 -0400 Subject: [PATCH 08/10] refactor(dashboard): deduplicate workflow filter and hot-poll helpers --- src/app/components/dashboard/ActionsTab.tsx | 31 +-------- .../components/dashboard/DashboardPage.tsx | 54 +++++----------- src/app/lib/filters.ts | 18 ++++++ src/app/services/api.ts | 8 +-- src/app/services/poll.ts | 64 +++++++++---------- 5 files changed, 67 insertions(+), 108 deletions(-) diff --git a/src/app/components/dashboard/ActionsTab.tsx b/src/app/components/dashboard/ActionsTab.tsx index d7472ec3..4ea3adc1 100644 --- a/src/app/components/dashboard/ActionsTab.tsx +++ b/src/app/components/dashboard/ActionsTab.tsx @@ -3,7 +3,7 @@ import { createStore } from "solid-js/store"; import type { WorkflowRun } from "../../services/api"; import { viewState, setViewState, ignoreItem, unignoreItem, toggleExpandedRepo, setAllExpanded, isRepoExpanded, pruneExpandedRepos, pruneLockedRepos, ActionsFiltersSchema } from "../../stores/view"; import { createTabFilterHandlers, mergeActiveFilters } from "../../lib/tabFilters"; -import { isRunVisible, isWorkflowRunPending, isWorkflowRunWaiting } from "../../lib/filters"; +import { isRunVisible, isWorkflowRunPending, isWorkflowRunWaiting, matchesActionsConclusionFilter } from "../../lib/filters"; import WorkflowSummaryCard from "./WorkflowSummaryCard"; import IgnoreBadge from "./IgnoreBadge"; import SkeletonRows from "../shared/SkeletonRows"; @@ -167,33 +167,8 @@ export default function ActionsTab(props: ActionsTabProps) { return props.workflowRuns.filter((run) => { if (!isRunVisible(run, { ignoredIds, showPrRuns: viewState.showPrRuns, globalFilter })) return false; - if (conclusionFilter !== "all") { - if (conclusionFilter === "running") { - if (run.status !== "in_progress") return false; - } else if (conclusionFilter === "waiting") { - if (!isWorkflowRunWaiting(run)) return false; - } else if (conclusionFilter === "pending") { - if (!isWorkflowRunPending(run)) return false; - } else if (conclusionFilter === "other") { - if ( - run.status === "in_progress" || - isWorkflowRunWaiting(run) || - isWorkflowRunPending(run) || - run.conclusion === null || - (KNOWN_CONCLUSIONS as readonly string[]).includes(run.conclusion) - ) { - return false; - } - } else { - if ( - run.status === "in_progress" || - isWorkflowRunWaiting(run) || - isWorkflowRunPending(run) || - run.conclusion !== conclusionFilter - ) { - return false; - } - } + if (!matchesActionsConclusionFilter(run, conclusionFilter, KNOWN_CONCLUSIONS)) { + return false; } if (eventFilter !== "all") { diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 264a0973..6d611163 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -45,7 +45,7 @@ import { canApprovePR, canMarkPRReady, formatCount, prSizeCategory, rateLimitCss import { setsEqual } from "../../lib/collections"; import { withScrollLock } from "../../lib/scroll"; import { Tooltip } from "../shared/Tooltip"; -import { isIssueVisible, isPrVisible, isRunVisible, isWorkflowRunPending, isWorkflowRunWaiting } from "../../lib/filters"; +import { isIssueVisible, isPrVisible, isRunVisible, matchesActionsConclusionFilter } from "../../lib/filters"; import { isUserInvolved } from "../../lib/grouping"; import { KNOWN_CONCLUSIONS, KNOWN_EVENTS } from "../shared/filterTypes"; import CustomTabModal from "../shared/CustomTabModal"; @@ -576,14 +576,7 @@ async function pollFetch(force = false): Promise { }); } const filteredRuns = config.enableActions ? data.workflowRuns : []; - const hotPullRequests = - recentlyMarkedReadyPRs.size > 0 || recentlyApprovedPRs.size > 0 - ? resolvedPullRequests.map((pr) => - (recentlyMarkedReadyPRs.has(pr.id) || recentlyApprovedPRs.has(pr.id)) && pr.nodeId - ? { ...pr, enriched: true, checkStatus: "running" as const, hotPollPinned: true } - : pr - ) - : resolvedPullRequests; + const hotPullRequests = pinRecentlyMutatedPRs(resolvedPullRequests); rebuildHotSets({ ...data, pullRequests: hotPullRequests, workflowRuns: filteredRuns }); // Persist for stale-while-revalidate on full page reload. // Errors are transient and not persisted. Deferred to avoid blocking paint. @@ -758,18 +751,22 @@ function handleTargetedData(data: DashboardData, affectedRepos: string[]): void const newItems = detectNewItems(data); dispatchNotifications(newItems, config); - const hotTargetedPullRequests = - recentlyMarkedReadyPRs.size > 0 || recentlyApprovedPRs.size > 0 - ? data.pullRequests.map((pr) => - (recentlyMarkedReadyPRs.has(pr.id) || recentlyApprovedPRs.has(pr.id)) && pr.nodeId - ? { ...pr, enriched: true, checkStatus: "running" as const, hotPollPinned: true } - : pr - ) - : data.pullRequests; + const hotTargetedPullRequests = pinRecentlyMutatedPRs(data.pullRequests); seedHotSetsFromTargeted({ ...data, pullRequests: hotTargetedPullRequests, workflowRuns: targetedRuns }); persistDashboardSnapshot(); } +function pinRecentlyMutatedPRs(pullRequests: PullRequest[]): PullRequest[] { + if (recentlyMarkedReadyPRs.size === 0 && recentlyApprovedPRs.size === 0) { + return pullRequests; + } + return pullRequests.map((pr) => + (recentlyMarkedReadyPRs.has(pr.id) || recentlyApprovedPRs.has(pr.id)) && pr.nodeId + ? { ...pr, enriched: true, checkStatus: "running" as const, hotPollPinned: true } + : pr + ); +} + export default function DashboardPage() { const [hotPollingPRIds, setHotPollingPRIds] = createSignal>(new Set()); const [hotPollingRunIds, setHotPollingRunIds] = createSignal>(new Set()); @@ -1378,28 +1375,7 @@ export default function DashboardPage() { customCounts[tab.id] = data.workflowRuns.filter((w) => { if (!isItemVisibleOnTab(ownership.actions, w.id, tab.id)) return false; if (!isRunVisible(w, { ignoredIds: ignoredRuns, showPrRuns: viewState.showPrRuns, globalFilter: null })) return false; - if (f.conclusion !== "all") { - if (f.conclusion === "running") { if (w.status !== "in_progress") return false; } - else if (f.conclusion === "waiting") { if (!isWorkflowRunWaiting(w)) return false; } - else if (f.conclusion === "pending") { if (!isWorkflowRunPending(w)) return false; } - else if (f.conclusion === "other") { - if ( - w.status === "in_progress" || - isWorkflowRunWaiting(w) || - isWorkflowRunPending(w) || - w.conclusion === null || - (KNOWN_CONCLUSIONS as readonly string[]).includes(w.conclusion) - ) { - return false; - } - } - else if ( - w.status === "in_progress" || - isWorkflowRunWaiting(w) || - isWorkflowRunPending(w) || - w.conclusion !== f.conclusion - ) return false; - } + if (!matchesActionsConclusionFilter(w, f.conclusion, KNOWN_CONCLUSIONS)) return false; if (f.event !== "all") { if (f.event === "other") { if ((KNOWN_EVENTS as readonly string[]).includes(w.event)) return false; } else if (w.event !== f.event) return false; diff --git a/src/app/lib/filters.ts b/src/app/lib/filters.ts index 5e46d9cd..f8751e9a 100644 --- a/src/app/lib/filters.ts +++ b/src/app/lib/filters.ts @@ -51,3 +51,21 @@ export function isWorkflowRunWaiting(run: Pick): boolean { return run.status === "queued" || run.status === "pending" || run.status === "requested"; } + +export function matchesActionsConclusionFilter( + run: Pick, + conclusionFilter: string, + knownConclusions: readonly string[] +): boolean { + if (conclusionFilter === "all") return true; + if (conclusionFilter === "running") return run.status === "in_progress"; + if (conclusionFilter === "waiting") return isWorkflowRunWaiting(run); + if (conclusionFilter === "pending") return isWorkflowRunPending(run); + if (run.status === "in_progress" || isWorkflowRunWaiting(run) || isWorkflowRunPending(run)) { + return false; + } + if (conclusionFilter === "other") { + return run.conclusion !== null && !knownConclusions.includes(run.conclusion); + } + return run.conclusion === conclusionFilter; +} diff --git a/src/app/services/api.ts b/src/app/services/api.ts index fbf5d94e..6b46730d 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1148,10 +1148,6 @@ export interface PREnrichmentData { viewerLatestReviewState: ViewerReviewState | null; } -function extractRollup(node: { statusCheckRollup?: GraphQLStatusCheckRollup | null }): GraphQLStatusCheckRollup | null { - return node.statusCheckRollup ?? null; -} - function mapHeavyPRNode(node: GraphQLHeavyPRNode, isLightFallback: boolean): PREnrichmentData { const pendingLogins = (node.reviewRequests?.nodes ?? []) .map((n) => n?.requestedReviewer?.login) @@ -1165,7 +1161,7 @@ function mapHeavyPRNode(node: GraphQLHeavyPRNode, isLightFallback: boolean): PRE ? null : applyMergeStateOverride( node.mergeStateStatus ?? "", - mapCheckStatus(extractRollup(node)), + mapCheckStatus(node.statusCheckRollup ?? null), ); let headRepository: PREnrichmentData["headRepository"] = null; @@ -2508,7 +2504,7 @@ export async function fetchHotPRStatus( // Fork PRs whose CI runs only in the fork report no PR-level rollup; read the // fork head commit's rollup instead (the same source runForkPRFallback uses). const rollup = - extractRollup(node) ?? + node.statusCheckRollup ?? (node.isCrossRepository ? (node.headRef?.target?.statusCheckRollup ?? null) : null); const checkStatus = applyMergeStateOverride(node.mergeStateStatus, mapCheckStatus(rollup)); diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index 9343b3b6..170d8380 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -595,6 +595,31 @@ export function createPollCoordinator( * is active. Increments the generation counter so stale hot poll results * from the previous cycle can be detected and discarded. */ +function pruneExpiredHotPRPins(now: number): void { + for (const [id, untilMs] of _hotPRPinnedUntil) { + if (untilMs <= now) _hotPRPinnedUntil.delete(id); + } +} + +function syncHotPRPinAndCheckEligibility(pr: PullRequest, now: number): boolean { + const isTerminal = pr.state === "CLOSED" || pr.state === "MERGED"; + if (!isTerminal && pr.hotPollPinned) { + if ((_hotPRPinnedUntil.get(pr.id) ?? 0) <= now) { + _hotPRPinnedUntil.set(pr.id, now + HOT_PR_MUTATION_PIN_TTL_MS); + } + } else if (_hotPRPinnedUntil.has(pr.id)) { + _hotPRPinnedUntil.delete(pr.id); + } + + return ( + !isTerminal && + Boolean(pr.enriched && pr.nodeId) && + (pr.checkStatus === "running" || + pr.checkStatusStale === true || + (_hotPRPinnedUntil.get(pr.id) ?? 0) > now) + ); +} + export function rebuildHotSets(data: DashboardData): void { _hotPollGeneration++; _hotPRs.clear(); @@ -602,26 +627,10 @@ export function rebuildHotSets(data: DashboardData): void { _hotRuns.clear(); const now = Date.now(); - for (const [id, untilMs] of _hotPRPinnedUntil) { - if (untilMs <= now) _hotPRPinnedUntil.delete(id); - } + pruneExpiredHotPRPins(now); for (const pr of data.pullRequests) { - if (pr.hotPollPinned) { - if ((_hotPRPinnedUntil.get(pr.id) ?? 0) <= now) { - _hotPRPinnedUntil.set(pr.id, now + HOT_PR_MUTATION_PIN_TTL_MS); - } - } else if (_hotPRPinnedUntil.has(pr.id)) { - _hotPRPinnedUntil.delete(pr.id); - } - - if ( - pr.enriched && - pr.nodeId && - (pr.checkStatus === "running" || - pr.checkStatusStale === true || - (_hotPRPinnedUntil.get(pr.id) ?? 0) > now) - ) { + if (syncHotPRPinAndCheckEligibility(pr, now) && pr.nodeId) { if (_hotPRs.size >= MAX_HOT_PRS) { console.warn(`[hot-poll] PR cap reached (${MAX_HOT_PRS}), skipping remaining`); break; @@ -996,26 +1005,11 @@ export async function fetchTargetedRepoData( */ export function seedHotSetsFromTargeted(data: DashboardData): void { const now = Date.now(); - for (const [id, untilMs] of _hotPRPinnedUntil) { - if (untilMs <= now) _hotPRPinnedUntil.delete(id); - } + pruneExpiredHotPRPins(now); for (const pr of data.pullRequests) { const isTerminal = pr.state === "CLOSED" || pr.state === "MERGED"; - if (!isTerminal && pr.hotPollPinned) { - if ((_hotPRPinnedUntil.get(pr.id) ?? 0) <= now) { - _hotPRPinnedUntil.set(pr.id, now + HOT_PR_MUTATION_PIN_TTL_MS); - } - } else { - _hotPRPinnedUntil.delete(pr.id); - } - - const isEligible = - !isTerminal && - Boolean(pr.enriched && pr.nodeId) && - (pr.checkStatus === "running" || - pr.checkStatusStale === true || - (_hotPRPinnedUntil.get(pr.id) ?? 0) > now); + const isEligible = syncHotPRPinAndCheckEligibility(pr, now); if (isEligible && pr.nodeId) { if (!_hotPRs.has(pr.nodeId) && _hotPRs.size < MAX_HOT_PRS) { From f0a8cebd76ae3336abf06cd43ad9d6a890055b19 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 31 Oct 2026 08:02:43 -0400 Subject: [PATCH 09/10] test(checks): add unit tests for workflow filters and fork fallback Cover isWorkflowRunWaiting, isWorkflowRunPending, and matchesActionsConclusionFilter in tests/lib/filters.test.ts (including non-terminal exclusion on terminal conclusion filters and action_required exclusion under conclusion="other") and verify 3-way rollup classification and partial-error salvage in runForkPRFallback in tests/services/api.test.ts. --- tests/lib/filters.test.ts | 117 +++++++++++++++++++++++++++++++- tests/services/api.test.ts | 135 +++++++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 2 deletions(-) diff --git a/tests/lib/filters.test.ts b/tests/lib/filters.test.ts index 2d2f66b4..50582958 100644 --- a/tests/lib/filters.test.ts +++ b/tests/lib/filters.test.ts @@ -1,8 +1,16 @@ import { describe, it, expect } from "vitest"; -import { isIssueVisible, isPrVisible, isRunVisible } from "../../src/app/lib/filters"; +import { + isIssueVisible, + isPrVisible, + isRunVisible, + isWorkflowRunPending, + isWorkflowRunWaiting, + matchesActionsConclusionFilter, +} from "../../src/app/lib/filters"; +import { KNOWN_CONCLUSIONS } from "../../src/app/components/shared/filterTypes"; import { makeIssue, makePullRequest, makeWorkflowRun } from "../helpers/index"; -// All three predicates are pure functions — no reactive root or store setup needed. +// All predicates are pure functions — no reactive root or store setup needed. describe("isIssueVisible", () => { describe("ignoredIds", () => { @@ -234,3 +242,108 @@ describe("isRunVisible", () => { }); }); }); + +describe("isWorkflowRunWaiting", () => { + it("returns true for waiting and action_required statuses and completed action_required conclusion", () => { + expect(isWorkflowRunWaiting({ status: "waiting", conclusion: null })).toBe(true); + expect(isWorkflowRunWaiting({ status: "action_required", conclusion: null })).toBe(true); + expect(isWorkflowRunWaiting({ status: "completed", conclusion: "action_required" })).toBe(true); + }); + + it("returns false for running, pending, and other completed conclusions", () => { + expect(isWorkflowRunWaiting({ status: "in_progress", conclusion: null })).toBe(false); + expect(isWorkflowRunWaiting({ status: "queued", conclusion: null })).toBe(false); + expect(isWorkflowRunWaiting({ status: "pending", conclusion: null })).toBe(false); + expect(isWorkflowRunWaiting({ status: "requested", conclusion: null })).toBe(false); + expect(isWorkflowRunWaiting({ status: "completed", conclusion: "success" })).toBe(false); + expect(isWorkflowRunWaiting({ status: "completed", conclusion: "failure" })).toBe(false); + expect(isWorkflowRunWaiting({ status: "completed", conclusion: null })).toBe(false); + }); +}); + +describe("isWorkflowRunPending", () => { + it("returns true for queued, pending, and requested statuses", () => { + expect(isWorkflowRunPending({ status: "queued" })).toBe(true); + expect(isWorkflowRunPending({ status: "pending" })).toBe(true); + expect(isWorkflowRunPending({ status: "requested" })).toBe(true); + }); + + it("returns false for running, waiting, action_required, and completed statuses", () => { + expect(isWorkflowRunPending({ status: "in_progress" })).toBe(false); + expect(isWorkflowRunPending({ status: "waiting" })).toBe(false); + expect(isWorkflowRunPending({ status: "action_required" })).toBe(false); + expect(isWorkflowRunPending({ status: "completed" })).toBe(false); + }); +}); + +describe("matchesActionsConclusionFilter", () => { + it("matches all runs when conclusionFilter is 'all'", () => { + expect(matchesActionsConclusionFilter({ status: "in_progress", conclusion: null }, "all", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "waiting", conclusion: null }, "all", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "queued", conclusion: null }, "all", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "success" }, "all", KNOWN_CONCLUSIONS)).toBe(true); + }); + + it("matches only in_progress runs when conclusionFilter is 'running'", () => { + expect(matchesActionsConclusionFilter({ status: "in_progress", conclusion: null }, "running", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "waiting", conclusion: null }, "running", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "queued", conclusion: null }, "running", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "success" }, "running", KNOWN_CONCLUSIONS)).toBe(false); + }); + + it("matches waiting and action_required runs when conclusionFilter is 'waiting'", () => { + expect(matchesActionsConclusionFilter({ status: "waiting", conclusion: null }, "waiting", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "action_required", conclusion: null }, "waiting", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "action_required" }, "waiting", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "in_progress", conclusion: null }, "waiting", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "queued", conclusion: null }, "waiting", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "failure" }, "waiting", KNOWN_CONCLUSIONS)).toBe(false); + }); + + it("matches queued, pending, and requested runs when conclusionFilter is 'pending'", () => { + expect(matchesActionsConclusionFilter({ status: "queued", conclusion: null }, "pending", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "pending", conclusion: null }, "pending", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "requested", conclusion: null }, "pending", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "in_progress", conclusion: null }, "pending", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "waiting", conclusion: null }, "pending", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "success" }, "pending", KNOWN_CONCLUSIONS)).toBe(false); + }); + + it("excludes non-terminal and waiting runs from terminal conclusion filters ('success', 'failure', 'cancelled', 'other')", () => { + const nonTerminalRuns = [ + { status: "in_progress", conclusion: "success" }, + { status: "in_progress", conclusion: "skipped" }, + { status: "waiting", conclusion: "failure" }, + { status: "action_required", conclusion: "cancelled" }, + { status: "completed", conclusion: "action_required" }, + { status: "queued", conclusion: "success" }, + { status: "pending", conclusion: "failure" }, + { status: "requested", conclusion: "skipped" }, + ]; + + for (const run of nonTerminalRuns) { + expect(matchesActionsConclusionFilter(run, "success", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter(run, "failure", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter(run, "cancelled", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter(run, "other", KNOWN_CONCLUSIONS)).toBe(false); + } + }); + + it("matches completed runs against exact terminal conclusion filters ('success', 'failure', 'cancelled')", () => { + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "success" }, "success", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "failure" }, "success", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "failure" }, "failure", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "cancelled" }, "cancelled", KNOWN_CONCLUSIONS)).toBe(true); + }); + + it("matches completed runs with unknown non-null conclusions under 'other' and excludes KNOWN_CONCLUSIONS including 'action_required'", () => { + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "skipped" }, "other", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "timed_out" }, "other", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "neutral" }, "other", KNOWN_CONCLUSIONS)).toBe(true); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: null }, "other", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "success" }, "other", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "failure" }, "other", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "cancelled" }, "other", KNOWN_CONCLUSIONS)).toBe(false); + expect(matchesActionsConclusionFilter({ status: "completed", conclusion: "action_required" }, "other", KNOWN_CONCLUSIONS)).toBe(false); + }); +}); diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 0f4481ca..e25b5d7d 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -2840,6 +2840,141 @@ describe("fetchIssuesAndPullRequests — settled-PR enrichment cache", () => { expect(res2.pullRequests[0].checkStatusStale).toBeUndefined(); }); + it("classifies 3-way non-terminal rollups ('running', 'waiting', 'pending') in runForkPRFallback, selects STATUS_CHECK_ROLLUP_FIELDS, and salvages partial fork responses on error", async () => { + let throwPartialForkError = false; + const forkQueries: string[] = []; + + const lightForkPRs = [ + makeLightPR({ id: "PR_fork_1", databaseId: 401, number: 401 }), + makeLightPR({ id: "PR_fork_2", databaseId: 402, number: 402 }), + makeLightPR({ id: "PR_fork_3", databaseId: 403, number: 403 }), + ]; + + const heavyForkPRs = [401, 402, 403].map((dbId, idx) => + makeHeavyPR({ + id: `PR_fork_${idx + 1}`, + databaseId: dbId, + headRefOid: `sha-fork-${dbId}`, + headRepository: { owner: { login: `fork-user-${idx + 1}` }, nameWithOwner: `fork-user-${idx + 1}/Hello-World` }, + statusCheckRollup: null, + }) + ); + + const octokit = makeOctokit( + async () => ({ data: {} }), + async (query: string, variables?: unknown) => { + const vars = (variables ?? {}) as Record; + if ("ids" in vars) { + return { nodes: heavyForkPRs }; + } + if (query.includes("fork0:")) { + forkQueries.push(query); + if (throwPartialForkError) { + throw Object.assign(new Error("Partial fork GraphQL error"), { + data: { + fork0: { + object: { + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, + }, + }, + fork1: { + object: { + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "IN_PROGRESS", count: 2 }] }, + }, + }, + }, + fork2: null, + rateLimit: { cost: 1, limit: 5000, remaining: 4950, resetAt: "2026-04-23T12:00:00Z" }, + }, + }); + } + return { + fork0: { + object: { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 1 }, + { state: "WAITING", count: 1 }, + ], + }, + }, + }, + }, + fork1: { + object: { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 0 }, + { state: "WAITING", count: 1 }, + ], + }, + }, + }, + }, + fork2: { + object: { + statusCheckRollup: { + state: "PENDING", + contexts: { + checkRunCountsByState: [ + { state: "IN_PROGRESS", count: 0 }, + { state: "WAITING", count: 0 }, + { state: "QUEUED", count: 1 }, + ], + }, + }, + }, + }, + rateLimit: { cost: 1, limit: 5000, remaining: 4951, resetAt: "2026-04-23T12:00:00Z" }, + }; + } + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prInvolves: { issueCount: 3, pageInfo: { hasNextPage: false, endCursor: null }, nodes: lightForkPRs }, + prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + }; + } + ); + + // 1. Normal response path: selects STATUS_CHECK_ROLLUP_FIELDS and classifies running / waiting / pending + const resNormal = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + expect(forkQueries).toHaveLength(1); + expect(forkQueries[0]).toContain( + "statusCheckRollup { state contexts { checkRunCountsByState { state count } } }" + ); + expect(resNormal.pullRequests.find((p) => p.id === 401)?.checkStatus).toBe("running"); + expect(resNormal.pullRequests.find((p) => p.id === 402)?.checkStatus).toBe("waiting"); + expect(resNormal.pullRequests.find((p) => p.id === 403)?.checkStatus).toBe("pending"); + + // 2. Partial-error salvage path: salvages fork0 ("waiting") and fork1 ("running"), marks missing fork2 stale + throwPartialForkError = true; + vi.mocked(pushNotification).mockClear(); + const resPartial = await fetchIssuesAndPullRequests(octokit as never, [testRepo], "octocat"); + const pr401 = resPartial.pullRequests.find((p) => p.id === 401)!; + const pr402 = resPartial.pullRequests.find((p) => p.id === 402)!; + const pr403 = resPartial.pullRequests.find((p) => p.id === 403)!; + expect(pr401.checkStatus).toBe("waiting"); + expect(pr401.checkStatusStale).toBeUndefined(); + expect(pr402.checkStatus).toBe("running"); + expect(pr402.checkStatusStale).toBeUndefined(); + expect(pr403.checkStatus).toBeNull(); + expect(pr403.checkStatusStale).toBe(true); + expect(pushNotification).toHaveBeenCalledWith( + "graphql", + "Fork PR check status unavailable — CI status may be missing for some PRs", + "warning" + ); + }); + it("skips runForkPRFallback when a fork PR falls back to LIGHT_PR_BACKFILL_QUERY (checkStatusStale: true)", async () => { let forkCalls = 0; From d799e0762c2800af19b3e032651a8cd2c0a3e5e3 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 1 Nov 2026 09:36:48 -0500 Subject: [PATCH 10/10] feat(poll): hot-poll pending PRs for a bounded grace window PRs whose checks are "pending" (queued check runs or a pending commit status) now keep hot-polling for 3 minutes from the first pending observation on their head commit. This catches runner pickup, gaps between dependent jobs, and commit-status CI that reports pending while building, while PRs parked on a condition stop hot-polling after the window instead of indefinitely. --- README.md | 2 +- docs/USER_GUIDE.md | 4 +- src/app/services/poll.ts | 97 +++++++++++++---- tests/services/hot-poll.test.ts | 181 +++++++++++++++++++++++--------- 4 files changed, 216 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index c0d41e91..4ba3adb8 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ When you sign in, the app searches for repos you've interacted with that aren't ### Hot Polling -A second, faster poll loop (default 30s, configurable 10–120s) targets only actively running items — PRs with actively running (`Checks running`) or stale CI checks (plus a 60s window after inline `Approve` or `Ready?` actions) and in-progress/queued workflow runs, while skipping checks paused on human action (`Pending action`) or conditions (`Pending condition`). These are updated via minimal GraphQL `nodes()` queries and individual REST calls rather than full re-fetches, keeping API usage low during active development. +A second, faster poll loop (default 30s, configurable 10–120s) targets only actively running items — PRs with actively running (`Checks running`) or stale CI checks (plus a 60s window after inline `Approve` or `Ready?` actions, and the first 3 minutes of a `Pending condition` stretch to catch queued jobs) and in-progress/queued workflow runs, while skipping checks paused on human action (`Pending action`) or parked on a condition beyond that window. These are updated via minimal GraphQL `nodes()` queries and individual REST calls rather than full re-fetches, keeping API usage low during active development. ### Desktop Notifications diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index d2a153e1..40b71c0b 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -216,6 +216,8 @@ A small colored dot shows the aggregate CI status for the PR's latest commit: The dot links to the PR's checks page on GitHub. When checks are paused on human action or a pending condition, the PR row also displays an inline `Pending action` or `Pending condition` badge (in both comfortable and compact density across the Pull Requests and Tracked tabs). +CI systems that report through commit statuses rather than GitHub check runs (for example Jenkins, CircleCI, Buildkite, or Prow) mark a status as `pending` both while a build runs and while it waits on a condition, and GitHub's summary counts can't tell those apart. The app shows these as `Pending condition`. They are still re-checked every hot-poll cycle for the first 3 minutes of each pending stretch (see [Hot Poll](#hot-poll)), and the full refresh picks up the result after that. + #### Review Badges | Badge | Meaning | @@ -423,7 +425,7 @@ Each full refresh also checks GitHub's own status page for outages affecting Act A second, faster poll loop runs alongside the full refresh specifically for in-flight items. It targets: -- **PRs with actively running (`running`) or stale (`checkStatusStale`) CI checks** — re-checks status until checks leave the running state (plus a 60-second window after an inline `Approve` or `Ready?` action to catch newly triggered workflows; PRs paused in `waiting` or `pending` are not hot-polled). For PRs from forks whose CI runs only in the fork, the status is read from the fork's head commit, the same source the full refresh uses. +- **PRs with actively running (`running`) or stale (`checkStatusStale`) CI checks** — re-checks status until checks leave the running state (plus a 60-second window after an inline `Approve` or `Ready?` action to catch newly triggered workflows). PRs in `pending` (`Pending condition`) are re-checked for up to 3 minutes from when they were first seen pending on their current commit, which covers jobs waiting for a runner and gaps between dependent jobs; after that, PRs parked on a condition (such as review or draft status) wait for the next full refresh. A new commit or a return to `running` starts a fresh 3-minute window. PRs paused in `waiting` (`Pending action`) are not hot-polled. For PRs from forks whose CI runs only in the fork, the status is read from the fork's head commit, the same source the full refresh uses. - **In-progress or queued workflow runs** — re-checks until the run completes or pauses on an environment approval gate (`waiting`) Default interval: **30 seconds** (configurable from 10 to 120 seconds in Settings). diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index 170d8380..d3319cdd 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -57,7 +57,7 @@ export interface PollCoordinator { // ── Hot poll state ──────────────────────────────────────────────────────────── -/** PRs with running/stale/pinned check status: maps GraphQL node ID → databaseId */ +/** PRs with running/stale/pinned/pending-grace check status: maps GraphQL node ID → databaseId */ const _hotPRs = new Map(); /** Inverse index for O(1) eviction: maps databaseId → nodeId */ const _hotPRsByDbId = new Map(); @@ -65,6 +65,15 @@ const MAX_HOT_PRS = 200; const HOT_PR_MUTATION_PIN_TTL_MS = 60_000; /** Post-mutation pin deadlines: maps databaseId → expiry timestamp (ms) */ const _hotPRPinnedUntil = new Map(); +/** + * How long a PR whose checks are "pending" (queued, or a pending commit status) keeps + * hot-polling. Covers runner pickup, gaps between dependent jobs, and commit-status CI + * that reports "pending" while building, without hot-polling PRs parked on a condition + * (review, draft, labels) indefinitely. + */ +export const HOT_PR_PENDING_GRACE_MS = 180_000; +/** First observation of a continuous "pending" streak: maps databaseId → head commit + timestamp (ms) */ +const _hotPRPendingSince = new Map(); /** In-progress workflow runs: maps run ID → repo descriptor */ const _hotRuns = new Map(); @@ -91,6 +100,7 @@ export function clearHotSets(): void { _hotPRsByDbId.clear(); _hotRuns.clear(); _hotPRPinnedUntil.clear(); + _hotPRPendingSince.clear(); } export const WAKE_GRACE_MS = 2_000; @@ -587,20 +597,38 @@ export function createPollCoordinator( // ── Hot poll: targeted refresh for in-flight items ─────────────────────────── -/** - * Rebuilds hot item sets from fresh full refresh data. Called after each full - * poll cycle completes. Clears and replaces both sets (full replacement, not - * incremental). Enrolls PRs whose checkStatus is actively "running", whose - * checkStatusStale flag is set, or whose post-mutation hotPollPinned window - * is active. Increments the generation counter so stale hot poll results - * from the previous cycle can be detected and discarded. - */ function pruneExpiredHotPRPins(now: number): void { for (const [id, untilMs] of _hotPRPinnedUntil) { if (untilMs <= now) _hotPRPinnedUntil.delete(id); } } +/** + * Records a "pending" observation for a PR and reports whether it is still within + * HOT_PR_PENDING_GRACE_MS of the start of its current pending streak. Any other status + * ends the streak. A new head commit starts a new streak; an unknown ("") head SHA + * keeps the current one. + */ +function isWithinPendingGrace( + id: number, + checkStatus: PullRequest["checkStatus"], + headSha: string | undefined, + now: number, +): boolean { + if (checkStatus !== "pending") { + _hotPRPendingSince.delete(id); + return false; + } + const sha = headSha ?? ""; + const streak = _hotPRPendingSince.get(id); + if (!streak || (sha !== "" && streak.headSha !== "" && streak.headSha !== sha)) { + _hotPRPendingSince.set(id, { headSha: sha || (streak?.headSha ?? ""), since: now }); + return true; + } + if (streak.headSha === "" && sha !== "") streak.headSha = sha; + return now - streak.since < HOT_PR_PENDING_GRACE_MS; +} + function syncHotPRPinAndCheckEligibility(pr: PullRequest, now: number): boolean { const isTerminal = pr.state === "CLOSED" || pr.state === "MERGED"; if (!isTerminal && pr.hotPollPinned) { @@ -611,15 +639,34 @@ function syncHotPRPinAndCheckEligibility(pr: PullRequest, now: number): boolean _hotPRPinnedUntil.delete(pr.id); } + // Terminal PRs and post-mutation clones (whose checkStatus is synthetic) end any + // pending streak; stale or unenriched PRs carry no fresh status and leave it untouched. + let inPendingGrace = false; + if (isTerminal || pr.hotPollPinned) { + _hotPRPendingSince.delete(pr.id); + } else if (pr.enriched && pr.checkStatusStale !== true) { + inPendingGrace = isWithinPendingGrace(pr.id, pr.checkStatus, pr.headSha, now); + } + return ( !isTerminal && Boolean(pr.enriched && pr.nodeId) && (pr.checkStatus === "running" || pr.checkStatusStale === true || + inPendingGrace || (_hotPRPinnedUntil.get(pr.id) ?? 0) > now) ); } +/** + * Rebuilds hot item sets from fresh full refresh data. Called after each full + * poll cycle completes. Clears and replaces both sets (full replacement, not + * incremental). Enrolls PRs whose checkStatus is actively "running", whose + * checkStatusStale flag is set, whose post-mutation hotPollPinned window is + * active, or whose "pending" streak is within HOT_PR_PENDING_GRACE_MS. + * Increments the generation counter so stale hot poll results from the + * previous cycle can be detected and discarded. + */ export function rebuildHotSets(data: DashboardData): void { _hotPollGeneration++; _hotPRs.clear(); @@ -628,6 +675,11 @@ export function rebuildHotSets(data: DashboardData): void { const now = Date.now(); pruneExpiredHotPRPins(now); + // Full refresh data is authoritative: drop pending streaks for PRs no longer present. + const presentIds = new Set(data.pullRequests.map((pr) => pr.id)); + for (const id of _hotPRPendingSince.keys()) { + if (!presentIds.has(id)) _hotPRPendingSince.delete(id); + } for (const pr of data.pullRequests) { if (syncHotPRPinAndCheckEligibility(pr, now) && pr.nodeId) { @@ -655,9 +707,10 @@ export function rebuildHotSets(data: DashboardData): void { } /** - * Fetches updated status for all hot items (running/stale/pinned PRs + in-progress/queued runs). - * Evicts items from the hot sets when they leave active execution (PR closed/merged or - * checkStatus !== "running" outside a post-mutation pin window; workflow run status no longer + * Fetches updated status for all hot items (running/stale/pinned/pending-grace PRs + + * in-progress/queued runs). Evicts items from the hot sets when they leave active execution + * (PR closed/merged, or checkStatus !== "running" outside a post-mutation pin window and + * outside a HOT_PR_PENDING_GRACE_MS "pending" streak; workflow run status no longer * "in_progress" or "queued", e.g. "waiting" or "completed"). Returns captured generation * alongside results so callers can detect staleness. * @@ -726,7 +779,8 @@ export async function fetchHotData(priorConsecutiveFailures = 0): Promise<{ // The freshly rebuilt sets are authoritative — evicting from them based on // stale fetch results would corrupt the new data. if (generation === _hotPollGeneration) { - // Evict PRs that are no longer actively running (unless within a 60s post-mutation pin window) + // Evict PRs that are no longer actively running, unless within a 60s post-mutation + // pin window or a HOT_PR_PENDING_GRACE_MS "pending" streak const now = Date.now(); for (const [databaseId, upd] of prUpdates) { const isTerminal = upd.state === "CLOSED" || upd.state === "MERGED"; @@ -735,7 +789,13 @@ export async function fetchHotData(priorConsecutiveFailures = 0): Promise<{ _hotPRPinnedUntil.delete(databaseId); } const isPinned = !isTerminal && now < pinnedUntil; - if (isTerminal || (upd.checkStatus !== "running" && !isPinned)) { + let inPendingGrace = false; + if (isTerminal) { + _hotPRPendingSince.delete(databaseId); + } else { + inPendingGrace = isWithinPendingGrace(databaseId, upd.checkStatus, upd.headSha, now); + } + if (isTerminal || (upd.checkStatus !== "running" && !isPinned && !inPendingGrace)) { const nodeId = _hotPRsByDbId.get(databaseId); if (nodeId) { _hotPRs.delete(nodeId); @@ -998,10 +1058,11 @@ export async function fetchTargetedRepoData( /** * Updates hot item sets incrementally from a targeted refresh or post-mutation seed. - * Enrolls eligible running, stale, or post-mutation pinned PRs and queued/in_progress - * workflow runs, and evicts already-enrolled items in the targeted payload that have - * transitioned out of active execution (e.g. to "waiting", "pending", "success", or - * CLOSED/MERGED) without disturbing hot items in unaffected repositories. + * Enrolls eligible running, stale, post-mutation pinned, or pending-grace PRs and + * queued/in_progress workflow runs, and evicts already-enrolled items in the targeted + * payload that have transitioned out of active execution (e.g. to "waiting", "success", + * "pending" past HOT_PR_PENDING_GRACE_MS, or CLOSED/MERGED) without disturbing hot items + * in unaffected repositories. */ export function seedHotSetsFromTargeted(data: DashboardData): void { const now = Date.now(); diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 929d8f5b..37bd818b 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -75,6 +75,7 @@ import { fetchHotData, createHotPollCoordinator, getHotPollGeneration, + HOT_PR_PENDING_GRACE_MS, type DashboardData, } from "../../src/app/services/poll"; @@ -514,7 +515,7 @@ describe("rebuildHotSets", () => { expect(getHotPollGeneration()).toBe(2); }); - it("populates hot PRs for enriched running checkStatus with nodeId and skips waiting/pending without hotPollPinned", async () => { + it("populates hot PRs for enriched running checkStatus or a fresh pending streak with nodeId and skips waiting without hotPollPinned", async () => { const octokit = makeOctokit(undefined, () => Promise.resolve({ nodes: [], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, @@ -530,16 +531,15 @@ describe("rebuildHotSets", () => { makePullRequest({ id: 4, checkStatus: "running", enriched: true }), // no nodeId — skipped makePullRequest({ id: 5, checkStatus: "running", enriched: false, nodeId: "PR_e" }), // not enriched — skipped makePullRequest({ id: 6, checkStatus: "waiting", enriched: true, nodeId: "PR_waiting" }), // waiting without pin — skipped - makePullRequest({ id: 7, checkStatus: "pending", enriched: true, nodeId: "PR_pending" }), // pending without pin — skipped + makePullRequest({ id: 7, checkStatus: "pending", enriched: true, nodeId: "PR_pending" }), // first pending observation — within grace ], }); await fetchHotData(); - // Verify graphql was called with only the 1 eligible node ID + // Verify graphql was called with only the 2 eligible node IDs expect(octokit.graphql).toHaveBeenCalledTimes(1); const calledIds = (octokit.graphql.mock.calls[0][1] as { ids: string[] }).ids; - expect(calledIds).toHaveLength(1); - expect(calledIds).toContain("PR_a"); + expect(calledIds).toEqual(["PR_a", "PR_pending"]); }); it("populates hot PRs when checkStatusStale is true even if checkStatus is null", async () => { @@ -1645,54 +1645,138 @@ describe("fetchHotData eviction edge cases", () => { expect(second.prUpdates.size).toBe(0); }); - it("evicts a running PR from _hotPRs when a hot-poll cycle transitions it to waiting or pending", async () => { - const graphqlFn = vi.fn(() => Promise.resolve({ - nodes: [ - { - databaseId: 10, - state: "OPEN", - mergeStateStatus: "CLEAN", - reviewDecision: null, - statusCheckRollup: { - state: "PENDING", - contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + it("evicts a running PR from _hotPRs immediately on waiting and after the pending grace window on pending", async () => { + vi.useFakeTimers(); + try { + const graphqlFn = vi.fn(() => Promise.resolve({ + nodes: [ + { + databaseId: 10, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "WAITING", count: 1 }] }, + }, }, - }, - { - databaseId: 11, - state: "OPEN", - mergeStateStatus: "CLEAN", - reviewDecision: null, - statusCheckRollup: { - state: "PENDING", - contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + { + databaseId: 11, + state: "OPEN", + mergeStateStatus: "CLEAN", + reviewDecision: null, + statusCheckRollup: { + state: "PENDING", + contexts: { checkRunCountsByState: [{ state: "QUEUED", count: 1 }] }, + }, }, - }, - ], - rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, - })); - const octokit = makeOctokit(undefined, graphqlFn); - mockGetClient.mockReturnValue(octokit); + ], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + const octokit = makeOctokit(undefined, graphqlFn); + mockGetClient.mockReturnValue(octokit); - rebuildHotSets({ - ...emptyData, - pullRequests: [ - makePullRequest({ id: 10, checkStatus: "running", enriched: true, nodeId: "PR_to_waiting" }), - makePullRequest({ id: 11, checkStatus: "running", enriched: true, nodeId: "PR_to_pending" }), - ], - }); + rebuildHotSets({ + ...emptyData, + pullRequests: [ + makePullRequest({ id: 10, checkStatus: "running", enriched: true, nodeId: "PR_to_waiting" }), + makePullRequest({ id: 11, checkStatus: "running", enriched: true, nodeId: "PR_to_pending" }), + ], + }); - const first = await fetchHotData(); - expect(first.prUpdates.get(10)!.checkStatus).toBe("waiting"); - expect(first.prUpdates.get(11)!.checkStatus).toBe("pending"); + const first = await fetchHotData(); + expect(first.prUpdates.get(10)!.checkStatus).toBe("waiting"); + expect(first.prUpdates.get(11)!.checkStatus).toBe("pending"); - graphqlFn.mockClear(); - const second = await fetchHotData(); - expect(second.prUpdates.size).toBe(0); - expect(graphqlFn).not.toHaveBeenCalled(); + // "waiting" is evicted at once; "pending" starts a grace streak and stays hot + graphqlFn.mockClear(); + vi.advanceTimersByTime(HOT_PR_PENDING_GRACE_MS - 1); + const second = await fetchHotData(); + expect(graphqlFn).toHaveBeenCalledTimes(1); + expect((graphqlFn.mock.calls[0] as unknown as [string, { ids: string[] }])[1].ids).toEqual(["PR_to_pending"]); + expect(second.prUpdates.get(11)!.checkStatus).toBe("pending"); + + // Grace window elapsed: the next pending observation evicts it + vi.advanceTimersByTime(1); + await fetchHotData(); + graphqlFn.mockClear(); + const fourth = await fetchHotData(); + expect(fourth.prUpdates.size).toBe(0); + expect(graphqlFn).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it("bounds pending hot-polling per head commit across rebuilds, restarting the streak on a new commit or after running", async () => { + vi.useFakeTimers(); + try { + const octokit = makeOctokit(undefined, () => Promise.resolve({ + nodes: [], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, + })); + mockGetClient.mockReturnValue(octokit); + const hotIds = async (): Promise => { + octokit.graphql.mockClear(); + await fetchHotData(); + if (octokit.graphql.mock.calls.length === 0) return []; + return (octokit.graphql.mock.calls[0][1] as { ids: string[] }).ids; + }; + const rebuildWith = (overrides: Parameters[0]) => + rebuildHotSets({ + ...emptyData, + pullRequests: [makePullRequest({ id: 40, enriched: true, nodeId: "PR_parked", ...overrides })], + }); + + rebuildWith({ checkStatus: "pending", headSha: "sha1" }); + expect(await hotIds()).toEqual(["PR_parked"]); + + // Still within the streak that began at the first observation + vi.advanceTimersByTime(HOT_PR_PENDING_GRACE_MS - 1); + rebuildWith({ checkStatus: "pending", headSha: "sha1" }); + expect(await hotIds()).toEqual(["PR_parked"]); + + // Parked on the same commit past the grace window: not re-enrolled by later full refreshes + vi.advanceTimersByTime(1); + rebuildWith({ checkStatus: "pending", headSha: "sha1" }); + expect(await hotIds()).toEqual([]); + vi.advanceTimersByTime(300_000); + rebuildWith({ checkStatus: "pending", headSha: "sha1" }); + expect(await hotIds()).toEqual([]); + + // An unknown head SHA keeps the existing streak + rebuildWith({ checkStatus: "pending", headSha: "" }); + expect(await hotIds()).toEqual([]); + + // A new head commit starts a new streak + rebuildWith({ checkStatus: "pending", headSha: "sha2" }); + expect(await hotIds()).toEqual(["PR_parked"]); + + // Expire it, then a running observation ends the streak and a later pending starts fresh + vi.advanceTimersByTime(HOT_PR_PENDING_GRACE_MS); + rebuildWith({ checkStatus: "pending", headSha: "sha2" }); + expect(await hotIds()).toEqual([]); + rebuildWith({ checkStatus: "running", headSha: "sha2" }); + rebuildWith({ checkStatus: "pending", headSha: "sha2" }); + expect(await hotIds()).toEqual(["PR_parked"]); + + // Stale observations carry no fresh status and leave the streak untouched + vi.advanceTimersByTime(HOT_PR_PENDING_GRACE_MS); + rebuildWith({ checkStatus: "running", checkStatusStale: true, headSha: "sha2" }); + expect(await hotIds()).toEqual(["PR_parked"]); // enrolled because stale + rebuildWith({ checkStatus: "pending", headSha: "sha2" }); + expect(await hotIds()).toEqual([]); + + // PRs missing from a full refresh drop their streak + rebuildHotSets(emptyData); + rebuildWith({ checkStatus: "pending", headSha: "sha2" }); + expect(await hotIds()).toEqual(["PR_parked"]); + } finally { + vi.useRealTimers(); + } }); - it("evicts an already-enrolled unpinned running PR when seedHotSetsFromTargeted observes it transition to waiting, pending, or success while preserving hot PRs in other repos", async () => { + it("evicts an already-enrolled unpinned running PR when seedHotSetsFromTargeted observes it transition to waiting or success, keeps fresh pending streaks, and preserves hot PRs in other repos", async () => { const octokit = makeOctokit(undefined, () => Promise.resolve({ nodes: [], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, @@ -1709,7 +1793,8 @@ describe("fetchHotData eviction edge cases", () => { ], }); - // Also verify seedHotSetsFromTargeted does NOT enroll brand-new waiting/pending PRs without hotPollPinned + // seedHotSetsFromTargeted does NOT enroll brand-new waiting PRs without hotPollPinned, + // but first-observed pending PRs (new or transitioning) start a grace streak seedHotSetsFromTargeted({ ...emptyData, pullRequests: [ @@ -1724,7 +1809,7 @@ describe("fetchHotData eviction edge cases", () => { await fetchHotData(); expect(octokit.graphql).toHaveBeenCalledTimes(1); const calledIds = (octokit.graphql.mock.calls[0][1] as { ids: string[] }).ids; - expect(calledIds).toEqual(["PR_other_repo"]); + expect(calledIds).toEqual(["PR_other_repo", "PR_now_pending", "PR_new_pending"]); }); it("evicts a workflow run from _hotRuns when it transitions from in_progress to waiting on a hot-poll cycle or via seedHotSetsFromTargeted", async () => {