From f24e4a0421bf8139b7781c300cdae478517c1632 Mon Sep 17 00:00:00 2001 From: Tom X Nguyen Date: Tue, 21 Apr 2026 19:06:03 +0700 Subject: [PATCH 1/8] feat(dashboard): add htmlUrl to PullCheckRun type Surface the GitHub check run URL in the type so the UI can link directly to the run on github.com. --- apps/dashboard/src/lib/github.types.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index 8f7e797..abc97df 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -251,6 +251,7 @@ export type PullCheckRun = { appAvatarUrl: string | null; outputTitle: string | null; startedAt: string | null; + htmlUrl: string | null; }; export type PullReview = { From ce1c6fb7d85b7e917f9df7e0266f82f4de270039 Mon Sep 17 00:00:00 2001 From: Tom X Nguyen Date: Tue, 21 Apr 2026 19:06:15 +0700 Subject: [PATCH 2/8] feat(dashboard): add rerunChecks server function with shared check-run helpers Extract deduplicateCheckRuns and isFailedCheckRun as pure shared helpers used by both computePullStatus and the new rerunChecks mutation. The rerunChecks server function calls GitHub's checks.rerequestRun API to re-trigger failed or all check runs for a given pull request. - RerunChecksInput extends PullFromRepoInput (project convention) - Refactors computePullStatus to use deduplicateCheckRuns - isFailedCheckRun mirrors the UI's getCheckRunStatus logic - Passes html_url through computePullStatus mapping --- apps/dashboard/src/lib/github.functions.ts | 107 +++++++++++++++++++-- 1 file changed, 99 insertions(+), 8 deletions(-) diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 6527c0c..2ca055c 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -722,6 +722,42 @@ export type CommandPaletteSearchInput = { perPage?: number; }; +// --------------------------------------------------------------------------- +// Check run helpers — shared between computePullStatus and rerunChecks +// --------------------------------------------------------------------------- + +type CheckRunPayload = { + id: number; + name: string; + status: string; + conclusion: string | null; +}; + +/** Deduplicate check runs by name — keep the most recent run (highest id) per name. */ +export function deduplicateCheckRuns( + checkRuns: T[], +): T[] { + const latestByName = new Map(); + for (const check of checkRuns) { + const existing = latestByName.get(check.name); + if (!existing || check.id > existing.id) { + latestByName.set(check.name, check); + } + } + return Array.from(latestByName.values()); +} + +/** Whether a completed check run counts as failed (mirrors the UI's `getCheckRunStatus`). */ +export function isFailedCheckRun(run: CheckRunPayload): boolean { + return ( + run.status === "completed" && + run.conclusion !== "success" && + run.conclusion !== "neutral" && + run.conclusion !== "skipped" && + run.conclusion !== null + ); +} + function clampPerPage(value: number | undefined, fallback = 30) { if (!Number.isFinite(value)) { return fallback; @@ -3712,14 +3748,7 @@ async function computePullStatus( const allCheckRuns = checksResponse?.data.check_runs ?? []; // Deduplicate by name — keep the most recent run (highest id) per check name - const latestByName = new Map(); - for (const check of allCheckRuns) { - const existing = latestByName.get(check.name); - if (!existing || check.id > existing.id) { - latestByName.set(check.name, check); - } - } - const checkRuns = Array.from(latestByName.values()); + const checkRuns = deduplicateCheckRuns(allCheckRuns); let passed = 0; let failed = 0; @@ -3812,6 +3841,7 @@ async function computePullStatus( appAvatarUrl: check.app?.owner?.avatar_url ?? null, outputTitle: check.output?.title ?? null, startedAt: check.started_at ?? null, + htmlUrl: check.html_url ?? null, })), mergeable: pull.mergeable, mergeableState: @@ -6034,6 +6064,67 @@ export const deleteBranch = createServerFn({ method: "POST" }) } }); +export type RerunChecksInput = PullFromRepoInput & { + /** Rerun only failed checks. When false, rerun all checks. */ + failedOnly: boolean; +}; + +export const rerunChecks = createServerFn({ method: "POST" }) + .inputValidator(identityValidator) + .handler(async ({ data }): Promise => { + const context = await getGitHubUserContextForRepository(data); + if (!context) { + return { ok: false, error: "Not authenticated" }; + } + + try { + // Fetch the PR to get the head SHA + const pr = await context.octokit.rest.pulls.get({ + owner: data.owner, + repo: data.repo, + pull_number: data.pullNumber, + }); + + // List check runs for the head SHA + const checksResponse = await context.octokit.rest.checks.listForRef({ + owner: data.owner, + repo: data.repo, + ref: pr.data.head.sha, + per_page: 100, + }); + + const checkRuns = checksResponse.data.check_runs ?? []; + + // Deduplicate: keep the latest run per check name + const dedupedRuns = deduplicateCheckRuns(checkRuns); + + // Determine which checks to rerun + const runsToRerun = data.failedOnly + ? dedupedRuns.filter(isFailedCheckRun) + : dedupedRuns; + + if (runsToRerun.length === 0) { + return { ok: true }; + } + + // Rerequest each check run in parallel + await Promise.all( + runsToRerun.map((run) => + context.octokit.rest.checks.rerequestRun({ + owner: data.owner, + repo: data.repo, + check_run_id: run.id, + }), + ), + ); + + await bustPullDetailCaches(context.session.user.id, data); + return { ok: true }; + } catch (error) { + return toMutationError("rerun checks", error); + } + }); + async function getPullFilesResult( context: GitHubContext, data: PullFilesPageInput, From b8ee1d28b75484654a3ed0d851d2a56de9ab1982 Mon Sep 17 00:00:00 2001 From: Tom X Nguyen Date: Tue, 21 Apr 2026 19:06:24 +0700 Subject: [PATCH 3/8] feat(dashboard): add external links and rerun buttons to PR checks section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add external link icon (↗) to each check run row, linking to the run on github.com. Uses group-hover/run opacity pattern matching existing project conventions. - Add 'Re-run failed checks' button when checks have failures - Add 'Re-run all checks' button when any checks exist - Both buttons follow the ghost/xs/Spinner pattern used by UpdateBranchButton and MergeFooter - Pass owner/repo/pullNumber through ChecksSection props --- .../pulls/detail/pull-detail-activity.tsx | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx index cc87fd9..4323ee3 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -6,6 +6,7 @@ import { CommentIcon, Delete01Icon, EditIcon, + ExternalLinkIcon, GitBranchIcon, GitCommitIcon, GitMergeIcon, @@ -77,6 +78,7 @@ import { mergePullRequest, replyToReviewComment, requestPullReviewers, + rerunChecks, resolveReviewThread, unresolveReviewThread, updatePullBranch, @@ -495,6 +497,9 @@ function MergeStatusCard({ checkRuns={checkRuns} allChecksPassed={allChecksPassed} hasCheckFailures={hasCheckFailures} + owner={owner} + repo={repo} + pullNumber={pullNumber} /> )} @@ -739,13 +744,21 @@ function ChecksSection({ checkRuns, allChecksPassed, hasCheckFailures, + owner, + repo, + pullNumber, }: { checks: PullStatus["checks"]; checkRuns: PullCheckRun[]; allChecksPassed: boolean; hasCheckFailures: boolean; + owner: string; + repo: string; + pullNumber: number; }) { const [open, setOpen] = useState(true); + const [isRerunning, setIsRerunning] = useState(false); + const queryClient = useQueryClient(); const checkStatus: StatusType = allChecksPassed ? "success" @@ -784,6 +797,25 @@ function ChecksSection({ return order(a) - order(b); }); + const handleRerun = async (failedOnly: boolean) => { + setIsRerunning(true); + try { + const result = await rerunChecks({ + data: { owner, repo, pullNumber, failedOnly }, + }); + if (result.ok) { + await queryClient.invalidateQueries({ queryKey: ["github"] }); + } else { + toast.error(result.error); + checkPermissionWarning(result, `${owner}/${repo}`); + } + } catch { + toast.error("Failed to rerun checks"); + } finally { + setIsRerunning(false); + } + }; + return ( @@ -814,7 +846,7 @@ function ChecksSection({ return (
{run.appAvatarUrl && ( @@ -853,10 +885,53 @@ function ChecksSection({ ? "Pending" : "Skipped"} + {run.htmlUrl && ( + + + + )}
); })} + {(hasCheckFailures || checks.total > 0) && ( +
+ {hasCheckFailures && ( + + )} + {checks.total > 0 && ( + + )} +
+ )} ); From c0b63febb6875714dd40d9ecefdca478215ae1af Mon Sep 17 00:00:00 2001 From: Tom X Nguyen Date: Tue, 21 Apr 2026 19:06:34 +0700 Subject: [PATCH 4/8] test(dashboard): add unit tests for deduplicateCheckRuns and isFailedCheckRun 15 tests covering deduplication by name/id, all failure conclusion variants (failure, timed_out, cancelled, action_required), and all non-failure conclusions (success, neutral, skipped, null). --- .../src/lib/github-check-runs.test.ts | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 apps/dashboard/src/lib/github-check-runs.test.ts diff --git a/apps/dashboard/src/lib/github-check-runs.test.ts b/apps/dashboard/src/lib/github-check-runs.test.ts new file mode 100644 index 0000000..d478de0 --- /dev/null +++ b/apps/dashboard/src/lib/github-check-runs.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from "vitest"; +import { deduplicateCheckRuns, isFailedCheckRun } from "./github.functions"; + +describe("deduplicateCheckRuns", () => { + it("returns an empty array when given no check runs", () => { + expect(deduplicateCheckRuns([])).toEqual([]); + }); + + it("returns a single run unchanged", () => { + const runs = [ + { id: 1, name: "build", status: "completed", conclusion: "success" }, + ]; + expect(deduplicateCheckRuns(runs)).toEqual(runs); + }); + + it("keeps the run with the highest id when names collide", () => { + const runs = [ + { id: 10, name: "test", status: "completed", conclusion: "failure" }, + { id: 20, name: "test", status: "completed", conclusion: "success" }, + { id: 5, name: "test", status: "completed", conclusion: "failure" }, + ]; + const result = deduplicateCheckRuns(runs); + expect(result).toHaveLength(1); + expect(result[0].id).toBe(20); + }); + + it("preserves runs with different names", () => { + const runs = [ + { id: 1, name: "lint", status: "completed", conclusion: "success" }, + { id: 2, name: "build", status: "completed", conclusion: "failure" }, + { id: 3, name: "test", status: "queued", conclusion: null }, + ]; + expect(deduplicateCheckRuns(runs)).toEqual(runs); + }); + + it("deduplicates only by name, not by conclusion", () => { + const runs = [ + { id: 100, name: "deploy", status: "completed", conclusion: "failure" }, + { id: 200, name: "deploy", status: "completed", conclusion: "success" }, + { id: 300, name: "deploy", status: "in_progress", conclusion: null }, + ]; + const result = deduplicateCheckRuns(runs); + expect(result).toHaveLength(1); + expect(result[0].id).toBe(300); + }); +}); + +describe("isFailedCheckRun", () => { + it("returns true for conclusion=failure", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "failure", + }), + ).toBe(true); + }); + + it("returns true for conclusion=timed_out", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "timed_out", + }), + ).toBe(true); + }); + + it("returns true for conclusion=cancelled", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "cancelled", + }), + ).toBe(true); + }); + + it("returns true for conclusion=action_required", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "action_required", + }), + ).toBe(true); + }); + + it("returns false for conclusion=success", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "success", + }), + ).toBe(false); + }); + + it("returns false for conclusion=neutral", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "neutral", + }), + ).toBe(false); + }); + + it("returns false for conclusion=skipped", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: "skipped", + }), + ).toBe(false); + }); + + it("returns false for null conclusion (still running)", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "completed", + conclusion: null, + }), + ).toBe(false); + }); + + it("returns false for in-progress runs", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "in_progress", + conclusion: null, + }), + ).toBe(false); + }); + + it("returns false for queued runs", () => { + expect( + isFailedCheckRun({ + id: 1, + name: "ci", + status: "queued", + conclusion: null, + }), + ).toBe(false); + }); +}); From 68e6fa5c27abfd27be40bcc41c6f82bc4d83799b Mon Sep 17 00:00:00 2001 From: Tom X Nguyen Date: Tue, 21 Apr 2026 19:54:18 +0700 Subject: [PATCH 5/8] fix(dashboard): address CodeRabbit review feedback - Paginate checks.listForRef in rerunChecks using listPaginatedGitHubItems to handle >100 check runs - Add aria-label and focus-visible opacity to check run external links for accessibility (keyboard/screen-reader) - Use prefersNoHover to show links on non-hover devices - Use isFailedCheckRun in computePullStatus tallying to keep failure semantics in sync with rerun filtering - Fix test description: null conclusion with completed status is not 'still running' --- .../pulls/detail/pull-detail-activity.tsx | 9 ++++++- .../src/lib/github-check-runs.test.ts | 2 +- apps/dashboard/src/lib/github.functions.ts | 27 ++++++++++++------- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx index 4323ee3..3640c0f 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -759,6 +759,7 @@ function ChecksSection({ const [open, setOpen] = useState(true); const [isRerunning, setIsRerunning] = useState(false); const queryClient = useQueryClient(); + const prefersNoHover = usePrefersNoHover(); const checkStatus: StatusType = allChecksPassed ? "success" @@ -890,7 +891,13 @@ function ChecksSection({ href={run.htmlUrl} target="_blank" rel="noopener noreferrer" - className="shrink-0 text-muted-foreground opacity-0 transition-opacity hover:text-foreground group-hover/run:opacity-100" + aria-label={`Open ${run.name} on GitHub`} + className={cn( + "shrink-0 text-muted-foreground transition-opacity hover:text-foreground", + prefersNoHover + ? "opacity-100" + : "opacity-0 group-hover/run:opacity-100 focus-visible:opacity-100", + )} > diff --git a/apps/dashboard/src/lib/github-check-runs.test.ts b/apps/dashboard/src/lib/github-check-runs.test.ts index d478de0..73d06e2 100644 --- a/apps/dashboard/src/lib/github-check-runs.test.ts +++ b/apps/dashboard/src/lib/github-check-runs.test.ts @@ -123,7 +123,7 @@ describe("isFailedCheckRun", () => { ).toBe(false); }); - it("returns false for null conclusion (still running)", () => { + it("returns false for null conclusion (completed)", () => { expect( isFailedCheckRun({ id: 1, diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 2ca055c..9d45be0 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -3764,7 +3764,7 @@ async function computePullStatus( passed += 1; } else if (check.conclusion === "skipped") { skipped += 1; - } else { + } else if (isFailedCheckRun(check)) { failed += 1; } } @@ -6085,16 +6085,25 @@ export const rerunChecks = createServerFn({ method: "POST" }) pull_number: data.pullNumber, }); - // List check runs for the head SHA - const checksResponse = await context.octokit.rest.checks.listForRef({ - owner: data.owner, - repo: data.repo, - ref: pr.data.head.sha, - per_page: 100, + // List all check runs for the head SHA (paginated) + type CheckRunItem = Awaited< + ReturnType + >["data"]["check_runs"][number]; + const checkRuns = await listPaginatedGitHubItems({ + label: `rerun checks ${data.owner}/${data.repo}#${data.pullNumber}`, + request: (page) => + context.octokit.rest.checks.listForRef({ + owner: data.owner, + repo: data.repo, + ref: pr.data.head.sha, + page, + per_page: 100, + }), + getItems: (payload) => + ((payload as { check_runs?: CheckRunItem[] }).check_runs ?? + []) as CheckRunItem[], }); - const checkRuns = checksResponse.data.check_runs ?? []; - // Deduplicate: keep the latest run per check name const dedupedRuns = deduplicateCheckRuns(checkRuns); From f541ae692030223cebe02625a44b4a4fb1af2b1f Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Wed, 22 Apr 2026 00:13:54 -0400 Subject: [PATCH 6/8] feat(dashboard): expand checks section with statuses, expected, and workflow approvals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fetch commit statuses (CodeRabbit/CircleCI) alongside check runs - Show required and "Expected" checks sourced from branch rulesets - Surface workflow-approval banner + approveWorkflowRuns server fn - Route reruns via actions API (reRunWorkflow / reRunWorkflowFailedJobs) for GitHub Actions jobs; per-run rerequest for other apps - Group the checks list by status (Expected, Failed, Pending, Skipped, Passed) - Extract reusable StatePill component for Open/Merged/Closed/Draft badges - Webhook signals for status, workflow_run → pullEntity, repository_ruleset/branch_protection → repoProtection - Distinguish 403-resource errors from 403-auth in mutation handling --- README.md | 5 +- .../issues/detail/issue-detail-header.tsx | 22 +- .../pulls/detail/pull-detail-activity.tsx | 297 +++++++++---- .../pulls/detail/pull-detail-header.tsx | 10 +- .../pulls/detail/pull-detail-page.tsx | 8 +- .../src/lib/github-cache-invalidation.test.ts | 54 +++ apps/dashboard/src/lib/github-cache-policy.ts | 4 + apps/dashboard/src/lib/github-revalidation.ts | 58 +++ apps/dashboard/src/lib/github.functions.ts | 400 ++++++++++++++++-- apps/dashboard/src/lib/github.types.ts | 9 + apps/dashboard/src/lib/pr-state.ts | 11 +- packages/ui/src/components/state-pill.tsx | 40 ++ 12 files changed, 765 insertions(+), 153 deletions(-) create mode 100644 packages/ui/src/components/state-pill.tsx diff --git a/README.md b/README.md index 19d9e22..dd77f87 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,10 @@ Expanding permissions after users have installed the app will require those inst | Pull request review | Yes | Review state and PR detail | | Pull request review comment | Yes | Diff discussion and review comments | | Pull request review thread | Yes | Review thread state changes | -| Workflow run | Later | For Actions dashboard (workflow-run updates) | +| Status | Yes | Commit statuses (CodeRabbit, CircleCI, etc.) on PR pages | +| Repository ruleset | Yes | Required status checks & "Expected" check rendering | +| Branch protection rule | Yes | Required status checks (legacy protection) | +| Workflow run | Yes | Workflow approval state + Actions dashboard | | Workflow job | Later | For Actions dashboard (job-level logs) | | Push | Later | Branch-aware activity features | | Repository | Later | Repo settings and metadata changes | diff --git a/apps/dashboard/src/components/issues/detail/issue-detail-header.tsx b/apps/dashboard/src/components/issues/detail/issue-detail-header.tsx index 8581a17..a7888dc 100644 --- a/apps/dashboard/src/components/issues/detail/issue-detail-header.tsx +++ b/apps/dashboard/src/components/issues/detail/issue-detail-header.tsx @@ -1,6 +1,9 @@ import { IssuesIcon } from "@diffkit/icons"; import { Markdown } from "@diffkit/ui/components/markdown"; -import { cn } from "@diffkit/ui/lib/utils"; +import { + StatePill, + type StatePillTone, +} from "@diffkit/ui/components/state-pill"; import { useState } from "react"; import { IssueCommentReactionBar } from "#/components/details/comment-reaction-bar"; import { DetailPageTitle } from "#/components/details/detail-page"; @@ -12,7 +15,7 @@ import { usePrefersNoHover } from "#/lib/use-prefers-no-hover"; type IssueStateConfig = { color: string; label: string; - badgeClass: string; + tone: StatePillTone; }; export function getIssueStateConfig(issue: IssueDetail): IssueStateConfig { @@ -21,19 +24,19 @@ export function getIssueStateConfig(issue: IssueDetail): IssueStateConfig { return { color: "text-muted-foreground", label: "Closed", - badgeClass: "bg-muted text-muted-foreground", + tone: "muted", }; } return { color: "text-purple-500", label: "Closed", - badgeClass: "bg-purple-500/10 text-purple-500", + tone: "merged", }; } return { color: "text-green-500", label: "Open", - badgeClass: "bg-green-500/10 text-green-500", + tone: "open", }; } @@ -67,14 +70,7 @@ export function IssueDetailHeader({ title={issue.title} subtitle={
- - {stateConfig.label} - + {stateConfig.label} {issue.author && ( 0; const allChecksPassed = - checks.total > 0 && checks.failed === 0 && checks.pending === 0; + checks.total > 0 && + checks.failed === 0 && + checks.pending === 0 && + checks.expected === 0; const hasCheckFailures = checks.failed > 0; const isBehind = behindBy !== null && behindBy > 0; const hasConflicts = mergeableState === "dirty"; @@ -491,10 +498,11 @@ function MergeStatusCard({ /> {/* Checks section */} - {checks.total > 0 && ( + {(checks.total > 0 || pendingWorkflowApprovals.length > 0) && ( 1 ? "s" : ""}` - : `${checks.pending} pending check${checks.pending > 1 ? "s" : ""}`; + : `${pendingTotal} pending check${pendingTotal > 1 ? "s" : ""}`; const parts: string[] = []; if (checks.skipped > 0) parts.push(`${checks.skipped} skipped`); @@ -779,24 +791,38 @@ function ChecksSection({ `${checks.passed} successful check${checks.passed !== 1 ? "s" : ""}`, ); if (checks.pending > 0) parts.push(`${checks.pending} pending`); + if (checks.expected > 0) parts.push(`${checks.expected} expected`); if (checks.failed > 0) parts.push(`${checks.failed} failing`); const description = parts.join(", "); - // Sort: failed first, then pending, then skipped, then passed - const sortedRuns = [...checkRuns].sort((a, b) => { - const order = (run: PullCheckRun) => { - if (run.status !== "completed") return 1; - if ( - run.conclusion === "failure" || - run.conclusion === "timed_out" || - run.conclusion === "cancelled" - ) - return 0; - if (run.conclusion === "skipped") return 2; - return 3; + // Group by status in display order: expected, failed, pending, skipped, passed + const groupedRuns = useMemo(() => { + const groups: Record< + "expected" | "failure" | "pending" | "skipped" | "success", + PullCheckRun[] + > = { + expected: [], + failure: [], + pending: [], + skipped: [], + success: [], }; - return order(a) - order(b); - }); + for (const run of checkRuns) { + groups[getCheckRunStatus(run)].push(run); + } + return groups; + }, [checkRuns]); + + const checkRunGroups: Array<{ + key: keyof typeof groupedRuns; + label: string; + }> = [ + { key: "expected", label: "Expected" }, + { key: "failure", label: "Failed" }, + { key: "pending", label: "Pending" }, + { key: "skipped", label: "Skipped" }, + { key: "success", label: "Passed" }, + ]; const handleRerun = async (failedOnly: boolean) => { setIsRerunning(true); @@ -805,6 +831,17 @@ function ChecksSection({ data: { owner, repo, pullNumber, failedOnly }, }); if (result.ok) { + const rerun = result.rerun ?? 0; + const skipped = result.skipped ?? 0; + if (rerun > 0 && skipped > 0) { + toast.success( + `Re-running ${rerun} check${rerun !== 1 ? "s" : ""} · ${skipped} not eligible`, + ); + } else if (rerun > 0) { + toast.success(`Re-running ${rerun} check${rerun !== 1 ? "s" : ""}`); + } else if (skipped > 0) { + toast.info("No checks are eligible for rerun"); + } await queryClient.invalidateQueries({ queryKey: ["github"] }); } else { toast.error(result.error); @@ -817,6 +854,30 @@ function ChecksSection({ } }; + const handleApprove = async () => { + setIsApproving(true); + try { + const result = await approveWorkflowRuns({ + data: { + owner, + repo, + pullNumber, + workflowRunIds: pendingWorkflowApprovals.map((a) => a.workflowRunId), + }, + }); + if (result.ok) { + await queryClient.invalidateQueries({ queryKey: ["github"] }); + } else { + toast.error(result.error); + checkPermissionWarning(result, `${owner}/${repo}`); + } + } catch { + toast.error("Failed to approve workflows"); + } finally { + setIsApproving(false); + } + }; + return ( @@ -840,68 +901,124 @@ function ChecksSection({
+ {pendingWorkflowApprovals.length > 0 && ( +
+
+ +
+
+

+ {pendingWorkflowApprovals.length} workflow + {pendingWorkflowApprovals.length !== 1 ? "s" : ""} awaiting + approval +

+

+ This workflow requires approval from a maintainer. +

+
+ +
+ )} -
- {sortedRuns.map((run) => { - const runStatus = getCheckRunStatus(run); +
+ {checkRunGroups.map(({ key, label }) => { + const runs = groupedRuns[key]; + if (runs.length === 0) return null; return ( -
- - {run.appAvatarUrl && ( - - )} - - {run.name} - - {runStatus === "pending" && run.startedAt - ? ` — Started ${formatRelativeTime(run.startedAt)}` - : run.outputTitle - ? ` — ${run.outputTitle}` - : null} - - - - {runStatus === "success" - ? "Passed" - : runStatus === "failure" - ? "Failed" - : runStatus === "pending" - ? "Pending" - : "Skipped"} - - {run.htmlUrl && ( - - - - )} +
+
+ {label} +
+ {runs.map((run) => { + const runStatus = getCheckRunStatus(run); + const detail = + runStatus === "expected" + ? "Waiting for status to be reported" + : runStatus === "pending" && run.startedAt + ? `Started ${formatRelativeTime(run.startedAt)}` + : run.outputTitle; + const nameContent = ( + <> + {run.name} + {detail && ( + + {" — "} + {detail} + + )} + + ); + return ( +
+ + {run.appAvatarUrl && ( + + )} + {run.htmlUrl ? ( + + {nameContent} + + ) : ( + + {nameContent} + + )} + {run.required && ( + + Required + + )} + + {runStatus === "success" + ? "Passed" + : runStatus === "failure" + ? "Failed" + : runStatus === "pending" + ? "Pending" + : runStatus === "expected" + ? "Expected" + : "Skipped"} + +
+ ); + })}
); })} @@ -932,6 +1049,13 @@ function ChecksSection({ size="xs" disabled={isRerunning} className="h-6 text-xs text-muted-foreground" + iconLeft={ + isRerunning ? ( + + ) : ( + + ) + } onClick={() => void handleRerun(false)} > Re-run all checks @@ -1378,7 +1502,7 @@ function StatusIcon({ status }: { status: StatusType }) { function CheckRunIcon({ status, }: { - status: "success" | "failure" | "pending" | "skipped"; + status: "success" | "failure" | "pending" | "skipped" | "expected"; }) { if (status === "success") { return ( @@ -1401,6 +1525,13 @@ function CheckRunIcon({
); } + if (status === "expected") { + return ( +
+
+
+ ); + } return (
- - {stateConfig.label} - + {stateConfig.label} {pr.author && ( <> { "workflowJob:stylessh/havana#202", ]); }); + + it("maps repository_ruleset events to repo protection signal", () => { + expect( + getGitHubWebhookRevalidationSignalKeys("repository_ruleset", { + repository: { + name: "havana", + owner: { login: "stylessh" }, + }, + }), + ).toEqual(["repoProtection:stylessh/havana"]); + }); + + it("maps branch_protection_rule events to repo protection signal", () => { + expect( + getGitHubWebhookRevalidationSignalKeys("branch_protection_rule", { + repository: { + name: "havana", + owner: { login: "stylessh" }, + }, + }), + ).toEqual(["repoProtection:stylessh/havana"]); + }); + + it("maps status events to repo statuses signal", () => { + expect( + getGitHubWebhookRevalidationSignalKeys("status", { + repository: { + name: "havana", + owner: { login: "stylessh" }, + }, + sha: "abc123", + }), + ).toEqual(["repoStatuses:stylessh/havana"]); + }); + + it("extracts pull signals from workflow_run payloads alongside run entity", () => { + expect( + getGitHubWebhookRevalidationSignalKeys("workflow_run", { + repository: { + name: "havana", + owner: { login: "stylessh" }, + }, + workflow_run: { + id: 55, + pull_requests: [{ number: 17 }, { number: 19 }], + }, + }), + ).toEqual([ + "actions:stylessh/havana", + "workflowRun:stylessh/havana#55", + "pull:stylessh/havana#17", + "pull:stylessh/havana#19", + ]); + }); }); diff --git a/apps/dashboard/src/lib/github-cache-policy.ts b/apps/dashboard/src/lib/github-cache-policy.ts index 1f63cfc..87e2194 100644 --- a/apps/dashboard/src/lib/github-cache-policy.ts +++ b/apps/dashboard/src/lib/github-cache-policy.ts @@ -48,4 +48,8 @@ export const githubCachePolicy = { staleTimeMs: 30 * 60 * 1000, gcTimeMs: 24 * 60 * 60 * 1000, }, + repoProtection: { + staleTimeMs: 30 * 60 * 1000, + gcTimeMs: 24 * 60 * 60 * 1000, + }, } as const; diff --git a/apps/dashboard/src/lib/github-revalidation.ts b/apps/dashboard/src/lib/github-revalidation.ts index c0dc9b9..c5cf3c5 100644 --- a/apps/dashboard/src/lib/github-revalidation.ts +++ b/apps/dashboard/src/lib/github-revalidation.ts @@ -22,6 +22,10 @@ export const githubRevalidationSignalKeys = { `workflowJob:${input.owner}/${input.repo}#${input.jobId}`, repoCode: (input: { owner: string; repo: string }) => `repoCode:${input.owner}/${input.repo}`, + repoProtection: (input: { owner: string; repo: string }) => + `repoProtection:${input.owner}/${input.repo}`, + repoStatuses: (input: { owner: string; repo: string }) => + `repoStatuses:${input.owner}/${input.repo}`, installationAccess: "installationAccess", } as const; @@ -158,6 +162,32 @@ function getCheckSuitePullSignals(payload: unknown) { }); } +function getWorkflowRunPullSignals(payload: unknown) { + const repository = getRepositoryIdentity(payload); + if (!repository || !isRecord(payload) || !isRecord(payload.workflow_run)) { + return []; + } + + const prs = payload.workflow_run.pull_requests; + if (!Array.isArray(prs)) { + return []; + } + + return prs.flatMap((pull) => { + if (!isRecord(pull) || typeof pull.number !== "number") { + return []; + } + + return [ + githubRevalidationSignalKeys.pullEntity({ + owner: repository.owner, + repo: repository.repo, + pullNumber: pull.number, + }), + ]; + }); +} + export function getGitHubWebhookRevalidationSignalKeys( event: string, payload: unknown, @@ -306,8 +336,34 @@ export function getGitHubWebhookRevalidationSignalKeys( return getCheckSuitePullSignals(payload); } + if ( + event === "repository_ruleset" || + event === "branch_protection_rule" || + event === "branch_protection_configuration" + ) { + return [ + githubRevalidationSignalKeys.repoProtection({ + owner: repository.owner, + repo: repository.repo, + }), + ]; + } + + // GitHub's `status` webhook payload has no pull_requests field, so we fan + // out to a repo-scoped signal. Any open PR page in the repo subscribes to + // this and re-fetches its status — captures CodeRabbit/CircleCI updates. + if (event === "status") { + return [ + githubRevalidationSignalKeys.repoStatuses({ + owner: repository.owner, + repo: repository.repo, + }), + ]; + } + if (event === "workflow_run") { const runId = getWorkflowRunId(payload); + const pullSignals = getWorkflowRunPullSignals(payload); return typeof runId === "number" ? [ githubRevalidationSignalKeys.actionsRepo({ @@ -319,12 +375,14 @@ export function getGitHubWebhookRevalidationSignalKeys( repo: repository.repo, runId, }), + ...pullSignals, ] : [ githubRevalidationSignalKeys.actionsRepo({ owner: repository.owner, repo: repository.repo, }), + ...pullSignals, ]; } diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 9d45be0..6dd8879 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -26,6 +26,7 @@ import type { NotificationsResult, OrgTeam, PinnedRepo, + PullCheckRun, PullComment, PullCommit, PullDetail, @@ -39,6 +40,7 @@ import type { PullReviewComment, PullStatus, PullSummary, + PullWorkflowApproval, ReplyToReviewCommentInput, RepoBranch, RepoCollaborator, @@ -652,10 +654,25 @@ export type MutationResult = | { ok: true } | { ok: false; error: string; installUrl?: string }; +/** GitHub sometimes returns 403 for resource-shape errors rather than auth — don't show the "install app" nudge for these. */ +function is403ResourceError(message: string): boolean { + return ( + /invalid check_run_id/i.test(message) || + /cannot be rerequested/i.test(message) || + /not eligible/i.test(message) + ); +} + function toMutationError(action: string, error: unknown): MutationResult { console.error(`[${action}]`, error); if (error instanceof RequestError) { + const msg = + (error.response?.data as { message?: string } | undefined)?.message ?? + error.message; if (error.status === 403) { + if (is403ResourceError(msg)) { + return { ok: false, error: `Failed to ${action}: ${msg}` }; + } return { ok: false, error: `Failed to ${action}: Insufficient permissions`, @@ -674,9 +691,6 @@ function toMutationError(action: string, error: unknown): MutationResult { error: `Failed to ${action}: Conflict — head branch may have been modified`, }; } - const msg = - (error.response?.data as { message?: string } | undefined)?.message ?? - error.message; return { ok: false, error: `Failed to ${action}: ${msg}` }; } return { ok: false, error: `Failed to ${action}: Unknown error` }; @@ -754,10 +768,69 @@ export function isFailedCheckRun(run: CheckRunPayload): boolean { run.conclusion !== "success" && run.conclusion !== "neutral" && run.conclusion !== "skipped" && + run.conclusion !== "stale" && run.conclusion !== null ); } +/** + * Fetch the set of required status check contexts for a branch, via GitHub's + * unified branch-rules endpoint (covers both rulesets and classic protection). + * Cached per repo — invalidated by repository_ruleset / branch_protection_rule webhooks. + */ +async function getRequiredStatusContexts( + context: GitHubContext, + params: { owner: string; repo: string; branch: string }, +): Promise { + return getOrRevalidateGitHubResource({ + userId: context.session.user.id, + resource: "repos.requiredStatusContexts", + params, + freshForMs: githubCachePolicy.repoProtection.staleTimeMs, + signalKeys: [ + githubRevalidationSignalKeys.repoProtection({ + owner: params.owner, + repo: params.repo, + }), + ], + fetcher: async () => { + const contexts = new Set(); + try { + const { data: rules } = await context.octokit.rest.repos.getBranchRules( + { + owner: params.owner, + repo: params.repo, + branch: params.branch, + }, + ); + for (const rule of rules) { + if (rule.type !== "required_status_checks") continue; + const parameters = (rule as { parameters?: unknown }).parameters; + if (!parameters || typeof parameters !== "object") continue; + const required = ( + parameters as { + required_status_checks?: Array<{ context?: unknown }>; + } + ).required_status_checks; + if (!Array.isArray(required)) continue; + for (const entry of required) { + if (entry && typeof entry.context === "string") { + contexts.add(entry.context); + } + } + } + } catch { + // Missing permission or unavailable — treat as no required contexts. + } + return { + kind: "success" as const, + data: Array.from(contexts), + metadata: createGitHubResponseMetadata(200, {}), + }; + }, + }); +} + function clampPerPage(value: number | undefined, fallback = 30) { if (!Number.isFinite(value)) { return fallback; @@ -3700,25 +3773,53 @@ async function computePullStatus( data: PullFromRepoInput, pull: RepoPullDetail, ): Promise { - const [reviewsResponse, checksResponse, userContext, oauthContext] = - await Promise.all([ - context.octokit.rest.pulls.listReviews({ + const [ + reviewsResponse, + checksResponse, + combinedStatusResponse, + workflowRunsResponse, + requiredContexts, + userContext, + oauthContext, + ] = await Promise.all([ + context.octokit.rest.pulls.listReviews({ + owner: data.owner, + repo: data.repo, + pull_number: data.pullNumber, + per_page: 100, + }), + context.octokit.rest.checks + .listForRef({ owner: data.owner, repo: data.repo, - pull_number: data.pullNumber, + ref: pull.head.sha, per_page: 100, - }), - context.octokit.rest.checks - .listForRef({ - owner: data.owner, - repo: data.repo, - ref: pull.head.sha, - per_page: 100, - }) - .catch(() => null), - getGitHubUserContextForRepository(data), - getGitHubContext(), - ]); + }) + .catch(() => null), + context.octokit.rest.repos + .getCombinedStatusForRef({ + owner: data.owner, + repo: data.repo, + ref: pull.head.sha, + per_page: 100, + }) + .catch(() => null), + context.octokit.rest.actions + .listWorkflowRunsForRepo({ + owner: data.owner, + repo: data.repo, + head_sha: pull.head.sha, + per_page: 100, + }) + .catch(() => null), + getRequiredStatusContexts(context, { + owner: data.owner, + repo: data.repo, + branch: pull.base.ref, + }).catch(() => []), + getGitHubUserContextForRepository(data), + getGitHubContext(), + ]); const permissions = mergeRepositoryPermissions( await getRepositoryPermissions( userContext ?? context, @@ -3749,26 +3850,112 @@ async function computePullStatus( // Deduplicate by name — keep the most recent run (highest id) per check name const checkRuns = deduplicateCheckRuns(allCheckRuns); + const requiredContextSet = new Set(requiredContexts); + + const mappedCheckRuns: PullCheckRun[] = checkRuns.map((check) => ({ + id: check.id, + name: check.name, + status: check.status, + conclusion: check.conclusion, + appAvatarUrl: check.app?.owner?.avatar_url ?? null, + outputTitle: check.output?.title ?? null, + startedAt: check.started_at ?? null, + htmlUrl: check.html_url ?? null, + required: requiredContextSet.has(check.name), + })); + + // Commit statuses (e.g. CodeRabbit, CircleCI) — separate from Check Runs. + // getCombinedStatusForRef already returns the latest status per context. + const combinedStatuses = combinedStatusResponse?.data.statuses ?? []; + const checkRunNames = new Set(mappedCheckRuns.map((run) => run.name)); + const mappedStatuses: PullCheckRun[] = combinedStatuses + .filter((status) => !checkRunNames.has(status.context)) + .map((status) => { + const state = status.state; + const isPending = state === "pending"; + return { + id: status.id, + name: status.context, + status: isPending ? "in_progress" : "completed", + conclusion: isPending + ? null + : state === "success" + ? "success" + : "failure", + appAvatarUrl: status.avatar_url ?? null, + outputTitle: status.description ?? null, + startedAt: status.created_at ?? null, + htmlUrl: status.target_url ?? null, + required: requiredContextSet.has(status.context), + }; + }); + + // Required contexts that haven't been reported yet → synthesize "expected" rows. + const reportedNames = new Set([ + ...mappedCheckRuns.map((run) => run.name), + ...mappedStatuses.map((run) => run.name), + ]); + const expectedChecks: PullCheckRun[] = requiredContexts + .filter((context) => !reportedNames.has(context)) + .map((contextName, index) => ({ + id: -1 - index, + name: contextName, + status: "expected", + conclusion: null, + appAvatarUrl: null, + outputTitle: null, + startedAt: null, + htmlUrl: null, + required: true, + })); + + const combinedChecks: PullCheckRun[] = [ + ...mappedCheckRuns, + ...mappedStatuses, + ...expectedChecks, + ]; let passed = 0; let failed = 0; let pending = 0; let skipped = 0; - for (const check of checkRuns) { - if (check.status !== "completed") { + let expected = 0; + for (const check of combinedChecks) { + if (check.status === "expected") { + expected += 1; + } else if (check.status !== "completed") { pending += 1; } else if ( check.conclusion === "success" || check.conclusion === "neutral" ) { passed += 1; - } else if (check.conclusion === "skipped") { + } else if (check.conclusion === "skipped" || check.conclusion === "stale") { skipped += 1; - } else if (isFailedCheckRun(check)) { + } else if (check.conclusion !== null) { failed += 1; } } + // Workflow runs awaiting approval — three shapes GitHub uses: + // - status=waiting → deployment environment awaits approval + // - status=action_required → run is blocked pre-start (e.g. during re-run) + // - status=completed, conclusion=action_required → first-time contributor gate + const pendingWorkflowApprovals: PullWorkflowApproval[] = ( + workflowRunsResponse?.data.workflow_runs ?? [] + ) + .filter( + (run) => + run.status === "waiting" || + run.status === "action_required" || + (run.status === "completed" && run.conclusion === "action_required"), + ) + .map((run) => ({ + workflowRunId: run.id, + name: run.name ?? `Workflow #${run.id}`, + event: run.event, + })); + let behindBy: number | null = null; let conflictingFiles: string[] = []; const hasConflicts = @@ -3827,22 +4014,15 @@ async function computePullStatus( return { reviews: Array.from(latestReviews.values()), checks: { - total: checkRuns.length, + total: combinedChecks.length, passed, failed, pending, skipped, + expected, }, - checkRuns: checkRuns.map((check) => ({ - id: check.id, - name: check.name, - status: check.status, - conclusion: check.conclusion, - appAvatarUrl: check.app?.owner?.avatar_url ?? null, - outputTitle: check.output?.title ?? null, - startedAt: check.started_at ?? null, - htmlUrl: check.html_url ?? null, - })), + checkRuns: combinedChecks, + pendingWorkflowApprovals, mergeable: pull.mergeable, mergeableState: typeof pull.mergeable_state === "string" ? pull.mergeable_state : null, @@ -3865,13 +4045,17 @@ async function getPullStatusResult( repo: data.repo, pullNumber: data.pullNumber, }); + const repoStatusesKey = githubRevalidationSignalKeys.repoStatuses({ + owner: data.owner, + repo: data.repo, + }); return getOrRevalidateGitHubResource({ userId: context.session.user.id, resource: "pulls.status.v3", params: data, freshForMs: githubCachePolicy.status.staleTimeMs, - signalKeys: [pullNamespaceKey], + signalKeys: [pullNamespaceKey, repoStatusesKey], namespaceKeys: [pullNamespaceKey], cacheMode: "split", fetcher: async () => { @@ -6069,9 +6253,16 @@ export type RerunChecksInput = PullFromRepoInput & { failedOnly: boolean; }; +export type RerunChecksResult = MutationResult & { + /** Number of checks that were successfully re-requested. */ + rerun?: number; + /** Number of checks GitHub refused to rerun because they're not eligible. */ + skipped?: number; +}; + export const rerunChecks = createServerFn({ method: "POST" }) .inputValidator(identityValidator) - .handler(async ({ data }): Promise => { + .handler(async ({ data }): Promise => { const context = await getGitHubUserContextForRepository(data); if (!context) { return { ok: false, error: "Not authenticated" }; @@ -6113,24 +6304,147 @@ export const rerunChecks = createServerFn({ method: "POST" }) : dedupedRuns; if (runsToRerun.length === 0) { - return { ok: true }; + return { ok: true, rerun: 0, skipped: 0 }; + } + + // Route each check run to the correct rerun endpoint: + // - GitHub Actions jobs: use actions.reRunWorkflow / reRunWorkflowFailedJobs + // (checks.rerequestRun requires the check to belong to the calling GitHub App) + // - Third-party apps (GitGuardian, CodeRabbit, etc.): can't be rerun by us — + // GitHub returns 403 because we're not the owning app. Mark as skipped. + const actionsRunIdByCheck = new Map(); + const actionsRunIds = new Set(); + const otherCheckRunIds: number[] = []; + + for (const run of runsToRerun) { + if (run.app?.slug === "github-actions") { + const match = run.html_url?.match(/\/actions\/runs\/(\d+)\//); + if (match) { + const workflowRunId = Number(match[1]); + actionsRunIdByCheck.set(run.id, workflowRunId); + actionsRunIds.add(workflowRunId); + continue; + } + } + otherCheckRunIds.push(run.id); + } + + type Task = { + /** Which run ids this task covers — for reporting counts back to UI. */ + coveredCheckRunIds: number[]; + promise: Promise; + }; + const tasks: Task[] = []; + + for (const workflowRunId of actionsRunIds) { + const coveredCheckRunIds = runsToRerun + .filter((r) => actionsRunIdByCheck.get(r.id) === workflowRunId) + .map((r) => r.id); + tasks.push({ + coveredCheckRunIds, + promise: data.failedOnly + ? context.octokit.rest.actions.reRunWorkflowFailedJobs({ + owner: data.owner, + repo: data.repo, + run_id: workflowRunId, + }) + : context.octokit.rest.actions.reRunWorkflow({ + owner: data.owner, + repo: data.repo, + run_id: workflowRunId, + }), + }); } - // Rerequest each check run in parallel - await Promise.all( - runsToRerun.map((run) => - context.octokit.rest.checks.rerequestRun({ + for (const checkRunId of otherCheckRunIds) { + tasks.push({ + coveredCheckRunIds: [checkRunId], + promise: context.octokit.rest.checks.rerequestRun({ owner: data.owner, repo: data.repo, - check_run_id: run.id, + check_run_id: checkRunId, + }), + }); + } + + const results = await Promise.allSettled(tasks.map((t) => t.promise)); + + let rerun = 0; + let skipped = 0; + const hardFailures: PromiseRejectedResult[] = []; + for (let i = 0; i < results.length; i++) { + const result = results[i]; + const covered = tasks[i].coveredCheckRunIds.length; + if (result.status === "fulfilled") { + rerun += covered; + continue; + } + const reason = result.reason; + const msg = + reason instanceof RequestError + ? ((reason.response?.data as { message?: string } | undefined) + ?.message ?? reason.message) + : ""; + if ( + reason instanceof RequestError && + reason.status === 403 && + is403ResourceError(msg) + ) { + skipped += covered; + } else { + hardFailures.push(result); + } + } + + if (rerun === 0 && hardFailures.length > 0) { + return toMutationError("rerun checks", hardFailures[0].reason); + } + + await bustPullDetailCaches(context.session.user.id, data); + return { ok: true, rerun, skipped }; + } catch (error) { + return toMutationError("rerun checks", error); + } + }); + +export type ApproveWorkflowRunsInput = PullFromRepoInput & { + workflowRunIds: number[]; +}; + +export const approveWorkflowRuns = createServerFn({ method: "POST" }) + .inputValidator(identityValidator) + .handler(async ({ data }): Promise => { + const context = await getGitHubUserContextForRepository(data); + if (!context) { + return { ok: false, error: "Not authenticated" }; + } + + if (data.workflowRunIds.length === 0) { + return { ok: true }; + } + + try { + const results = await Promise.allSettled( + data.workflowRunIds.map((runId) => + context.octokit.rest.actions.approveWorkflowRun({ + owner: data.owner, + repo: data.repo, + run_id: runId, }), ), ); + const rejected = results.filter( + (r): r is PromiseRejectedResult => r.status === "rejected", + ); + if (rejected.length === data.workflowRunIds.length) { + return toMutationError("approve workflow runs", rejected[0].reason); + } + await bustPullDetailCaches(context.session.user.id, data); return { ok: true }; } catch (error) { - return toMutationError("rerun checks", error); + return toMutationError("approve workflow runs", error); } }); diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index abc97df..d0b623a 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -252,6 +252,13 @@ export type PullCheckRun = { outputTitle: string | null; startedAt: string | null; htmlUrl: string | null; + required: boolean; +}; + +export type PullWorkflowApproval = { + workflowRunId: number; + name: string; + event: string; }; export type PullReview = { @@ -268,8 +275,10 @@ export type PullStatus = { failed: number; pending: number; skipped: number; + expected: number; }; checkRuns: PullCheckRun[]; + pendingWorkflowApprovals: PullWorkflowApproval[]; mergeable: boolean | null; mergeableState: string | null; conflictingFiles: string[]; diff --git a/apps/dashboard/src/lib/pr-state.ts b/apps/dashboard/src/lib/pr-state.ts index 5eee73d..08bc901 100644 --- a/apps/dashboard/src/lib/pr-state.ts +++ b/apps/dashboard/src/lib/pr-state.ts @@ -4,6 +4,7 @@ import { GitPullRequestDraftIcon, GitPullRequestIcon, } from "@diffkit/icons"; +import type { StatePillTone } from "@diffkit/ui/components/state-pill"; export type PrStateConfig = { icon: React.ComponentType<{ @@ -13,7 +14,7 @@ export type PrStateConfig = { }>; color: string; label: string; - badgeClass: string; + tone: StatePillTone; }; export function getPrStateConfig(pr: { @@ -27,7 +28,7 @@ export function getPrStateConfig(pr: { icon: GitPullRequestDraftIcon, color: "text-muted-foreground", label: "Draft", - badgeClass: "bg-muted text-muted-foreground", + tone: "muted", }; } if (pr.isMerged || pr.mergedAt || pr.state === "merged") { @@ -35,7 +36,7 @@ export function getPrStateConfig(pr: { icon: GitMergeIcon, color: "text-purple-500", label: "Merged", - badgeClass: "bg-purple-500/10 text-purple-500", + tone: "merged", }; } if (pr.state === "closed") { @@ -43,13 +44,13 @@ export function getPrStateConfig(pr: { icon: GitPullRequestClosedIcon, color: "text-red-500", label: "Closed", - badgeClass: "bg-red-500/10 text-red-500", + tone: "closed", }; } return { icon: GitPullRequestIcon, color: "text-green-500", label: "Open", - badgeClass: "bg-green-500/10 text-green-500", + tone: "open", }; } diff --git a/packages/ui/src/components/state-pill.tsx b/packages/ui/src/components/state-pill.tsx new file mode 100644 index 0000000..481e815 --- /dev/null +++ b/packages/ui/src/components/state-pill.tsx @@ -0,0 +1,40 @@ +import { cva, type VariantProps } from "class-variance-authority"; +import type * as React from "react"; + +import { cn } from "../lib/utils"; + +const statePillVariants = cva( + "inline-flex items-center gap-1 rounded-full px-2 py-0.5 text-xs font-medium whitespace-nowrap shrink-0", + { + variants: { + tone: { + open: "bg-green-500/10 text-green-500", + closed: "bg-red-500/10 text-red-500", + merged: "bg-purple-500/10 text-purple-500", + muted: "bg-muted text-muted-foreground", + secondary: "bg-secondary text-secondary-foreground", + }, + }, + defaultVariants: { + tone: "muted", + }, + }, +); + +export type StatePillTone = NonNullable< + VariantProps["tone"] +>; + +export function StatePill({ + className, + tone, + ...props +}: React.ComponentProps<"span"> & VariantProps) { + return ( + + ); +} From 49921a47376852fcbbd838ac0bc87fc71b22bc35 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Wed, 22 Apr 2026 00:32:25 -0400 Subject: [PATCH 7/8] fix(dashboard): address checks section review feedback - getCheckRunStatus: treat completed+null conclusion as pending (match backend) - computePullStatus: paginate checks.listForRef to cover PRs with >100 runs - rerunChecks: surface partial failures via { partial, failed } on result; UI shows a warning toast instead of a green success when some reruns hard-fail - ChecksSection: when checks.total === 0 but workflows await approval, render an approval-only title/description instead of "0 pending checks" - Approve workflows CTA: keep loading state until webhook drains the pending approvals list, with a 30s safety timeout - README: align Subscribe to events setup checklist with the webhook table (Status, Repository ruleset, Branch protection rule, Workflow run) --- README.md | 4 ++ .../pulls/detail/pull-detail-activity.tsx | 65 ++++++++++++++----- apps/dashboard/src/lib/github.functions.ts | 53 +++++++++++---- 3 files changed, 93 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index dd77f87..6387387 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,10 @@ The GitHub App provides installation tokens for repo-scoped access, webhook deli - Pull request review - Pull request review comment - Pull request review thread + - Status + - Repository ruleset + - Branch protection rule + - Workflow run 5. Click **Create GitHub App** diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx index 29ec1c3..7f6e0d8 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -50,6 +50,7 @@ import { lazy, Suspense, useCallback, + useEffect, useMemo, useRef, useState, @@ -772,6 +773,8 @@ function ChecksSection({ const queryClient = useQueryClient(); const pendingTotal = checks.pending + checks.expected; + const approvalCount = pendingWorkflowApprovals.length; + const isApprovalOnly = checks.total === 0 && approvalCount > 0; const checkStatus: StatusType = allChecksPassed ? "success" @@ -779,21 +782,28 @@ function ChecksSection({ ? "error" : "pending"; - const title = allChecksPassed - ? "All checks have passed" - : hasCheckFailures - ? `${checks.failed} failing check${checks.failed > 1 ? "s" : ""}` - : `${pendingTotal} pending check${pendingTotal > 1 ? "s" : ""}`; - - const parts: string[] = []; - if (checks.skipped > 0) parts.push(`${checks.skipped} skipped`); - parts.push( - `${checks.passed} successful check${checks.passed !== 1 ? "s" : ""}`, - ); - if (checks.pending > 0) parts.push(`${checks.pending} pending`); - if (checks.expected > 0) parts.push(`${checks.expected} expected`); - if (checks.failed > 0) parts.push(`${checks.failed} failing`); - const description = parts.join(", "); + const title = isApprovalOnly + ? `${approvalCount} workflow${approvalCount !== 1 ? "s" : ""} awaiting approval` + : allChecksPassed + ? "All checks have passed" + : hasCheckFailures + ? `${checks.failed} failing check${checks.failed > 1 ? "s" : ""}` + : `${pendingTotal} pending check${pendingTotal > 1 ? "s" : ""}`; + + let description: string; + if (isApprovalOnly) { + description = "Approve to run workflows and report checks."; + } else { + const parts: string[] = []; + if (checks.skipped > 0) parts.push(`${checks.skipped} skipped`); + parts.push( + `${checks.passed} successful check${checks.passed !== 1 ? "s" : ""}`, + ); + if (checks.pending > 0) parts.push(`${checks.pending} pending`); + if (checks.expected > 0) parts.push(`${checks.expected} expected`); + if (checks.failed > 0) parts.push(`${checks.failed} failing`); + description = parts.join(", "); + } // Group by status in display order: expected, failed, pending, skipped, passed const groupedRuns = useMemo(() => { @@ -833,7 +843,12 @@ function ChecksSection({ if (result.ok) { const rerun = result.rerun ?? 0; const skipped = result.skipped ?? 0; - if (rerun > 0 && skipped > 0) { + const failed = result.failed ?? 0; + if (result.partial) { + toast.warning( + `Re-running ${rerun} check${rerun !== 1 ? "s" : ""}, but ${failed} failed`, + ); + } else if (rerun > 0 && skipped > 0) { toast.success( `Re-running ${rerun} check${rerun !== 1 ? "s" : ""} · ${skipped} not eligible`, ); @@ -866,18 +881,32 @@ function ChecksSection({ }, }); if (result.ok) { + // Keep the button in loading state; the effect below resets it once the + // workflow_run webhook invalidates the cache and the pending list drains. await queryClient.invalidateQueries({ queryKey: ["github"] }); } else { toast.error(result.error); checkPermissionWarning(result, `${owner}/${repo}`); + setIsApproving(false); } } catch { toast.error("Failed to approve workflows"); - } finally { setIsApproving(false); } }; + // Reset the approving state when the pending list drains (webhook arrived) or + // after a safety timeout to avoid a permanently-stuck spinner. + useEffect(() => { + if (!isApproving) return; + if (pendingWorkflowApprovals.length === 0) { + setIsApproving(false); + return; + } + const timer = setTimeout(() => setIsApproving(false), 30_000); + return () => clearTimeout(timer); + }, [isApproving, pendingWorkflowApprovals.length]); + return ( @@ -1563,7 +1592,7 @@ function getCheckRunStatus( run: PullCheckRun, ): "success" | "failure" | "pending" | "skipped" | "expected" { if (run.status === "expected") return "expected"; - if (run.status !== "completed") return "pending"; + if (run.status !== "completed" || run.conclusion === null) return "pending"; if (run.conclusion === "success" || run.conclusion === "neutral") return "success"; if (run.conclusion === "skipped" || run.conclusion === "stale") diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 6dd8879..9aa3668 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -3773,9 +3773,13 @@ async function computePullStatus( data: PullFromRepoInput, pull: RepoPullDetail, ): Promise { + type CheckRunItem = Awaited< + ReturnType + >["data"]["check_runs"][number]; + const [ reviewsResponse, - checksResponse, + allCheckRuns, combinedStatusResponse, workflowRunsResponse, requiredContexts, @@ -3788,14 +3792,20 @@ async function computePullStatus( pull_number: data.pullNumber, per_page: 100, }), - context.octokit.rest.checks - .listForRef({ - owner: data.owner, - repo: data.repo, - ref: pull.head.sha, - per_page: 100, - }) - .catch(() => null), + listPaginatedGitHubItems({ + label: `pull status check runs ${data.owner}/${data.repo}#${data.pullNumber}`, + request: (page) => + context.octokit.rest.checks.listForRef({ + owner: data.owner, + repo: data.repo, + ref: pull.head.sha, + page, + per_page: 100, + }), + getItems: (payload) => + ((payload as { check_runs?: CheckRunItem[] }).check_runs ?? + []) as CheckRunItem[], + }).catch((): CheckRunItem[] => []), context.octokit.rest.repos .getCombinedStatusForRef({ owner: data.owner, @@ -3846,8 +3856,6 @@ async function computePullStatus( }); } - const allCheckRuns = checksResponse?.data.check_runs ?? []; - // Deduplicate by name — keep the most recent run (highest id) per check name const checkRuns = deduplicateCheckRuns(allCheckRuns); const requiredContextSet = new Set(requiredContexts); @@ -6258,6 +6266,10 @@ export type RerunChecksResult = MutationResult & { rerun?: number; /** Number of checks GitHub refused to rerun because they're not eligible. */ skipped?: number; + /** Number of checks that failed for non-permissions reasons (real errors). */ + failed?: number; + /** True when at least one rerun succeeded but some checks hit hard errors. */ + partial?: boolean; }; export const rerunChecks = createServerFn({ method: "POST" }) @@ -6396,11 +6408,30 @@ export const rerunChecks = createServerFn({ method: "POST" }) } } + const failed = hardFailures.reduce((total, failure) => { + const taskIdx = results.indexOf(failure); + return ( + total + (taskIdx >= 0 ? tasks[taskIdx].coveredCheckRunIds.length : 0) + ); + }, 0); + if (rerun === 0 && hardFailures.length > 0) { return toMutationError("rerun checks", hardFailures[0].reason); } + // Cache is still worth busting — the checks that did rerun updated state. await bustPullDetailCaches(context.session.user.id, data); + + if (hardFailures.length > 0) { + return { + ok: true, + rerun, + skipped, + failed, + partial: true, + }; + } + return { ok: true, rerun, skipped }; } catch (error) { return toMutationError("rerun checks", error); From 195ff0788481d81db016ff1a0381f07b484f0ea4 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Wed, 22 Apr 2026 12:51:05 -0400 Subject: [PATCH 8/8] fix(dashboard): address remaining CodeRabbit feedback on checks section - approveWorkflowRuns: surface partial failures via { approved, failed, partial } matching the rerunChecks shape; UI shows a warning toast on partial success - Broaden GHA workflow-run URL regex to accept /actions/runs/{id} OR /runs/{id} with `/`, `?`, or end-of-string terminators (still guarded by app.slug) - Paginate getCombinedStatusForRef + listWorkflowRunsForRepo in computePullStatus so large PRs don't truncate statuses or miss approval-awaiting workflow runs - Dedup combined statuses by context across pages --- .../pulls/detail/pull-detail-activity.tsx | 7 ++ apps/dashboard/src/lib/github.functions.ts | 107 +++++++++++++----- 2 files changed, 86 insertions(+), 28 deletions(-) diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx index 7f6e0d8..8960f26 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -881,6 +881,13 @@ function ChecksSection({ }, }); if (result.ok) { + if (result.partial) { + const approved = result.approved ?? 0; + const failed = result.failed ?? 0; + toast.warning( + `Approved ${approved} workflow${approved !== 1 ? "s" : ""}, but ${failed} failed`, + ); + } // Keep the button in loading state; the effect below resets it once the // workflow_run webhook invalidates the cache and the pending list drains. await queryClient.invalidateQueries({ queryKey: ["github"] }); diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 9aa3668..285766d 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -3776,12 +3776,18 @@ async function computePullStatus( type CheckRunItem = Awaited< ReturnType >["data"]["check_runs"][number]; + type CombinedStatusItem = Awaited< + ReturnType + >["data"]["statuses"][number]; + type WorkflowRunItem = Awaited< + ReturnType + >["data"]["workflow_runs"][number]; const [ reviewsResponse, allCheckRuns, - combinedStatusResponse, - workflowRunsResponse, + allCombinedStatuses, + allWorkflowRuns, requiredContexts, userContext, oauthContext, @@ -3806,22 +3812,34 @@ async function computePullStatus( ((payload as { check_runs?: CheckRunItem[] }).check_runs ?? []) as CheckRunItem[], }).catch((): CheckRunItem[] => []), - context.octokit.rest.repos - .getCombinedStatusForRef({ - owner: data.owner, - repo: data.repo, - ref: pull.head.sha, - per_page: 100, - }) - .catch(() => null), - context.octokit.rest.actions - .listWorkflowRunsForRepo({ - owner: data.owner, - repo: data.repo, - head_sha: pull.head.sha, - per_page: 100, - }) - .catch(() => null), + listPaginatedGitHubItems({ + label: `pull status combined statuses ${data.owner}/${data.repo}#${data.pullNumber}`, + request: (page) => + context.octokit.rest.repos.getCombinedStatusForRef({ + owner: data.owner, + repo: data.repo, + ref: pull.head.sha, + page, + per_page: 100, + }), + getItems: (payload) => + ((payload as { statuses?: CombinedStatusItem[] }).statuses ?? + []) as CombinedStatusItem[], + }).catch((): CombinedStatusItem[] => []), + listPaginatedGitHubItems({ + label: `pull status workflow runs ${data.owner}/${data.repo}#${data.pullNumber}`, + request: (page) => + context.octokit.rest.actions.listWorkflowRunsForRepo({ + owner: data.owner, + repo: data.repo, + head_sha: pull.head.sha, + page, + per_page: 100, + }), + getItems: (payload) => + ((payload as { workflow_runs?: WorkflowRunItem[] }).workflow_runs ?? + []) as WorkflowRunItem[], + }).catch((): WorkflowRunItem[] => []), getRequiredStatusContexts(context, { owner: data.owner, repo: data.repo, @@ -3873,8 +3891,17 @@ async function computePullStatus( })); // Commit statuses (e.g. CodeRabbit, CircleCI) — separate from Check Runs. - // getCombinedStatusForRef already returns the latest status per context. - const combinedStatuses = combinedStatusResponse?.data.statuses ?? []; + // GitHub's combined-status endpoint returns the latest status per context + // within each page; dedup across pages here in case the same context bleeds + // across page boundaries. + const combinedStatusesByContext = new Map(); + for (const status of allCombinedStatuses) { + const existing = combinedStatusesByContext.get(status.context); + if (!existing || status.id > existing.id) { + combinedStatusesByContext.set(status.context, status); + } + } + const combinedStatuses = Array.from(combinedStatusesByContext.values()); const checkRunNames = new Set(mappedCheckRuns.map((run) => run.name)); const mappedStatuses: PullCheckRun[] = combinedStatuses .filter((status) => !checkRunNames.has(status.context)) @@ -3949,9 +3976,7 @@ async function computePullStatus( // - status=waiting → deployment environment awaits approval // - status=action_required → run is blocked pre-start (e.g. during re-run) // - status=completed, conclusion=action_required → first-time contributor gate - const pendingWorkflowApprovals: PullWorkflowApproval[] = ( - workflowRunsResponse?.data.workflow_runs ?? [] - ) + const pendingWorkflowApprovals: PullWorkflowApproval[] = allWorkflowRuns .filter( (run) => run.status === "waiting" || @@ -6330,7 +6355,12 @@ export const rerunChecks = createServerFn({ method: "POST" }) for (const run of runsToRerun) { if (run.app?.slug === "github-actions") { - const match = run.html_url?.match(/\/actions\/runs\/(\d+)\//); + // GHA check URLs can be `/actions/runs/{id}/job/{job_id}` most of + // the time, but also plain `/actions/runs/{id}`, with a `?query`, or + // occasionally shortened to `/runs/{id}` — accept all shapes. + const match = run.html_url?.match( + /\/(?:actions\/runs|runs)\/(\d+)(?:\/|$|\?)/, + ); if (match) { const workflowRunId = Number(match[1]); actionsRunIdByCheck.set(run.id, workflowRunId); @@ -6442,16 +6472,25 @@ export type ApproveWorkflowRunsInput = PullFromRepoInput & { workflowRunIds: number[]; }; +export type ApproveWorkflowRunsResult = MutationResult & { + /** Number of workflow runs successfully approved. */ + approved?: number; + /** Number of workflow runs that failed to approve. */ + failed?: number; + /** True when at least one approval succeeded but some hit errors. */ + partial?: boolean; +}; + export const approveWorkflowRuns = createServerFn({ method: "POST" }) .inputValidator(identityValidator) - .handler(async ({ data }): Promise => { + .handler(async ({ data }): Promise => { const context = await getGitHubUserContextForRepository(data); if (!context) { return { ok: false, error: "Not authenticated" }; } if (data.workflowRunIds.length === 0) { - return { ok: true }; + return { ok: true, approved: 0, failed: 0 }; } try { @@ -6468,12 +6507,24 @@ export const approveWorkflowRuns = createServerFn({ method: "POST" }) const rejected = results.filter( (r): r is PromiseRejectedResult => r.status === "rejected", ); - if (rejected.length === data.workflowRunIds.length) { + const approved = results.length - rejected.length; + + if (approved === 0 && rejected.length > 0) { return toMutationError("approve workflow runs", rejected[0].reason); } await bustPullDetailCaches(context.session.user.id, data); - return { ok: true }; + + if (rejected.length > 0) { + return { + ok: true, + approved, + failed: rejected.length, + partial: true, + }; + } + + return { ok: true, approved, failed: 0 }; } catch (error) { return toMutationError("approve workflow runs", error); }