diff --git a/apps/web/components/github/pr-task-icon.test.ts b/apps/web/components/github/pr-task-icon.test.ts index 7155fc292e..21c67b5715 100644 --- a/apps/web/components/github/pr-task-icon.test.ts +++ b/apps/web/components/github/pr-task-icon.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import { aggregatePRStatusColor, + areAllOpenPRsReadyToMerge, getPRStatusColor, getPRTooltip, isPRAwaitingReview, @@ -421,6 +422,68 @@ describe("aggregatePRStatusColor", () => { const merged = makePR({ state: "merged" }); expect(aggregatePRStatusColor([merged, pending])).toBe("text-yellow-500"); }); + + it("ignores a merged PR when a fresh open PR has no status yet", () => { + // Repro: first PR merged, then a new branch + PR opened on the same task. + // The open PR has no checks/reviews yet so its color rank ties merged at + // 0, but the icon must reflect the live PR, not the closed one. + const merged = makePR({ state: "merged" }); + const fresh = makePR({ state: "open", review_state: "", checks_state: "" }); + expect(aggregatePRStatusColor([merged, fresh])).toBe("text-muted-foreground"); + }); + + it("ignores a closed PR when a fresh open PR is present", () => { + // Closed PRs rank red (5, the highest), so without filtering they would + // dominate every open sibling. Same reasoning as the merged case — a + // terminal PR shouldn't drive the live status indicator. + const closed = makePR({ state: "closed" }); + const fresh = makePR({ state: "open", review_state: "", checks_state: "" }); + expect(aggregatePRStatusColor([closed, fresh])).toBe("text-muted-foreground"); + }); + + it("falls back to terminal state when every PR is merged/closed", () => { + // All-terminal tasks bypass the open-PR filter and rank across every PR, + // so the result depends on which terminal state is present. Merged ranks + // 0 (purple), closed ranks 5 (red) — both paths need a guard. + const merged = makePR({ state: "merged" }); + expect(aggregatePRStatusColor([merged, merged])).toBe("text-purple-500"); + const closed = makePR({ state: "closed" }); + expect(aggregatePRStatusColor([closed, closed])).toBe("text-red-500"); + }); +}); + +describe("areAllOpenPRsReadyToMerge", () => { + const ready = () => + makePR({ + state: "open", + review_state: "approved", + checks_state: "success", + mergeable_state: "clean", + }); + + it("is false when the list is empty", () => { + expect(areAllOpenPRsReadyToMerge([])).toBe(false); + }); + + it("is false when no PR is open", () => { + // A fully-merged task isn't "ready to merge" — there's nothing left to do. + expect(areAllOpenPRsReadyToMerge([makePR({ state: "merged" })])).toBe(false); + }); + + it("is true when the only open PR is ready, even with a merged sibling", () => { + // The fix: a merged sibling used to drag the result to false because + // prs.every(isPRReadyToMerge) saw the merged PR as not-ready. + expect(areAllOpenPRsReadyToMerge([makePR({ state: "merged" }), ready()])).toBe(true); + }); + + it("is false when any open PR is not ready", () => { + const pending = makePR({ state: "open", checks_state: "pending" }); + expect(areAllOpenPRsReadyToMerge([ready(), pending])).toBe(false); + }); + + it("is true when every open PR is ready", () => { + expect(areAllOpenPRsReadyToMerge([ready(), ready()])).toBe(true); + }); }); describe("getPRTooltip", () => { diff --git a/apps/web/components/github/pr-task-icon.tsx b/apps/web/components/github/pr-task-icon.tsx index efe49feaa5..a7518587fc 100644 --- a/apps/web/components/github/pr-task-icon.tsx +++ b/apps/web/components/github/pr-task-icon.tsx @@ -89,13 +89,18 @@ export function getPRTooltip(pr: TaskPR): string { /** * Picks the most attention-worthy color across N PRs. For multi-repo tasks one - * red PR should dominate the visual even if the others are green. + * red PR should dominate the visual even if the others are green. Terminal + * (merged/closed) PRs are dropped when at least one PR is still open so a + * task whose first PR landed and was followed by a new open PR surfaces the + * live PR's status instead of the merged-purple from the closed one. */ export function aggregatePRStatusColor(prs: TaskPR[]): string { if (prs.length === 0) return "text-muted-foreground"; + const open = prs.filter((p) => p.state === "open"); + const target = open.length > 0 ? open : prs; let bestColor = "text-muted-foreground"; let bestRank = -1; - for (const pr of prs) { + for (const pr of target) { const color = getPRStatusColor(pr); const rank = STATUS_RANK[color] ?? 0; if (rank > bestRank) { @@ -106,6 +111,16 @@ export function aggregatePRStatusColor(prs: TaskPR[]): string { return bestColor; } +/** + * True when at least one PR is open AND every open PR is ready to merge. + * Terminal (merged/closed) siblings are ignored so they can't drag the result + * to false. Extracted so the rule is testable without mounting MultiPRIcon. + */ +export function areAllOpenPRsReadyToMerge(prs: TaskPR[]): boolean { + const openPRs = prs.filter((p) => p.state === "open"); + return openPRs.length > 0 && openPRs.every(isPRReadyToMerge); +} + export function PRTaskIcon({ taskId }: { taskId: string }) { const prs = useAppStore((state) => state.taskPRs.byTaskId[taskId] ?? null); @@ -138,7 +153,7 @@ function SinglePRIcon({ taskId, pr }: { taskId: string; pr: TaskPR }) { function MultiPRIcon({ taskId, prs }: { taskId: string; prs: TaskPR[] }) { const aggregateColor = aggregatePRStatusColor(prs); - const allReady = prs.every(isPRReadyToMerge); + const allReady = areAllOpenPRsReadyToMerge(prs); return (