From 19f0b686e9134767e7949ad69e5d0e4002b073b7 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Thu, 9 Apr 2026 14:12:34 -0400 Subject: [PATCH 1/3] Revamp MergeStatusCard with collapsible sections, merge actions, and permission warnings Rebuild the PR merge status card with GitHub-style collapsible sections for reviews and checks, individual check run display with app avatars, merge strategy dropdown, update branch split button, and a bottom bar warning system for insufficient GitHub App permissions. --- apps/dashboard/.dev.vars.example | 2 + .../layouts/dashboard-bottombar.tsx | 44 + .../components/layouts/dashboard-layout.tsx | 2 + .../pulls/detail/pull-detail-activity.tsx | 805 +++++++++++++++--- .../pulls/detail/pull-detail-sidebar.tsx | 2 + apps/dashboard/src/lib/github-app.server.ts | 4 + apps/dashboard/src/lib/github.functions.ts | 138 ++- apps/dashboard/src/lib/github.types.ts | 2 + apps/dashboard/src/lib/warning-store.ts | 73 ++ apps/dashboard/src/routes/__root.tsx | 2 + packages/ui/src/components/sonner.tsx | 1 + 11 files changed, 925 insertions(+), 150 deletions(-) create mode 100644 apps/dashboard/src/components/layouts/dashboard-bottombar.tsx create mode 100644 apps/dashboard/src/lib/warning-store.ts diff --git a/apps/dashboard/.dev.vars.example b/apps/dashboard/.dev.vars.example index 3fee5ea..6fe37be 100644 --- a/apps/dashboard/.dev.vars.example +++ b/apps/dashboard/.dev.vars.example @@ -6,6 +6,8 @@ # 5. Install the app on the repositories or organizations you want DiffKit to access GITHUB_APP_CLIENT_ID= GITHUB_APP_CLIENT_SECRET= +# The slug from your GitHub App URL (https://github.com/apps/) +GITHUB_APP_SLUG= # GitHub webhook secret used to verify deliveries to /api/webhooks/github # For local development, point your GitHub App webhook URL at a tunnel that forwards here. diff --git a/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx b/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx new file mode 100644 index 0000000..e6000c4 --- /dev/null +++ b/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx @@ -0,0 +1,44 @@ +import { AlertCircleIcon, XIcon } from "@diffkit/icons"; +import { cn } from "@diffkit/ui/lib/utils"; +import { removeWarning, useWarnings } from "#/lib/warning-store"; + +export function DashboardBottomBar() { + const warnings = useWarnings(); + + if (warnings.length === 0) return null; + + return ( +
+ {warnings.map((warning) => ( +
+ + {warning.message} + {warning.action && ( + + {warning.action.label} + + )} + {warning.dismissible && ( + + )} +
+ ))} +
+ ); +} diff --git a/apps/dashboard/src/components/layouts/dashboard-layout.tsx b/apps/dashboard/src/components/layouts/dashboard-layout.tsx index 46f5de9..008f854 100644 --- a/apps/dashboard/src/components/layouts/dashboard-layout.tsx +++ b/apps/dashboard/src/components/layouts/dashboard-layout.tsx @@ -7,6 +7,7 @@ import { } from "#/lib/github.query"; import { useGitHubRevalidation } from "#/lib/use-github-revalidation"; import { useHasMounted } from "#/lib/use-has-mounted"; +import { DashboardBottomBar } from "./dashboard-bottombar"; import { DashboardTopbar } from "./dashboard-topbar"; const routeApi = getRouteApi("/_protected"); @@ -57,6 +58,7 @@ export function DashboardLayout() { + ); 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 3dbee5b..6354ac7 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -1,6 +1,27 @@ -import { GitCommitIcon } from "@diffkit/icons"; +import { + CheckIcon, + ChevronDownIcon, + ChevronUpIcon, + GitCommitIcon, + GitMergeIcon, + MoreHorizontalIcon, + XIcon, +} from "@diffkit/icons"; +import { Button } from "@diffkit/ui/components/button"; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from "@diffkit/ui/components/collapsible"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@diffkit/ui/components/dropdown-menu"; import { Markdown } from "@diffkit/ui/components/markdown"; import { Skeleton } from "@diffkit/ui/components/skeleton"; +import { toast } from "@diffkit/ui/components/sonner"; import { cn } from "@diffkit/ui/lib/utils"; import { useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; @@ -9,13 +30,20 @@ import { DetailCommentBox, } from "#/components/details/detail-activity"; import { formatRelativeTime } from "#/lib/format-relative-time"; -import { updatePullBranch } from "#/lib/github.functions"; +import { + dismissPullReview, + mergePullRequest, + requestPullReviewers, + updatePullBranch, +} from "#/lib/github.functions"; import type { + PullCheckRun, PullComment, PullCommit, PullDetail, PullStatus, } from "#/lib/github.types"; +import { checkPermissionWarning } from "#/lib/warning-store"; export function PullDetailActivitySection({ comments, @@ -112,6 +140,7 @@ function MergeStatusCard({ }) { const { checks, + checkRuns, reviews, mergeable, mergeableState, @@ -119,84 +148,52 @@ function MergeStatusCard({ baseRefName, canUpdateBranch, } = status; - const [isUpdating, setIsUpdating] = useState(false); - const approvedReviews = reviews.filter( - (review) => review.state === "APPROVED", - ); + const approvedReviews = reviews.filter((r) => r.state === "APPROVED"); const changesRequested = reviews.filter( - (review) => review.state === "CHANGES_REQUESTED", - ); - const pendingReviewers = reviews.filter( - (review) => review.state === "PENDING", + (r) => r.state === "CHANGES_REQUESTED", ); - const hasReviewIssue = - changesRequested.length > 0 || pendingReviewers.length > 0; + const hasReviewIssue = changesRequested.length > 0; const allChecksPassed = checks.total > 0 && checks.failed === 0 && checks.pending === 0; const hasCheckFailures = checks.failed > 0; - const hasChecksPending = checks.pending > 0; const isBehind = behindBy !== null && behindBy > 0; + const hasConflicts = mergeableState === "dirty"; const isMergeBlocked = mergeableState === "blocked" || mergeable === false; return (
- 0 ? ( - - ) : approvedReviews.length > 0 && !hasReviewIssue ? ( - - ) : ( - - ) - } - title={ - changesRequested.length > 0 - ? "Changes requested" - : approvedReviews.length > 0 - ? `${approvedReviews.length} approving review${approvedReviews.length > 1 ? "s" : ""}` - : "Review required" - } - description={ - changesRequested.length > 0 - ? `${changesRequested.map((review) => review.author?.login).join(", ")} requested changes` - : approvedReviews.length > 0 && !hasReviewIssue - ? "All required reviews have been provided" - : "Code owner review required by reviewers with write access." - } + {/* Reviews section */} + + {/* Checks section */} {checks.total > 0 && ( - - ) : hasCheckFailures ? ( - - ) : ( - - ) - } - title={ - allChecksPassed - ? "All checks have passed" - : hasCheckFailures - ? `${checks.failed} failing check${checks.failed > 1 ? "s" : ""}` - : `${checks.pending} pending check${checks.pending > 1 ? "s" : ""}` - } - description={ - `${checks.skipped > 0 ? `${checks.skipped} skipped, ` : ""}${checks.passed} successful check${checks.passed !== 1 ? "s" : ""}` + - (hasChecksPending ? `, ${checks.pending} pending` : "") + - (hasCheckFailures ? `, ${checks.failed} failing` : "") - } + )} - {isBehind && ( + {/* Conflicts / branch status */} + {hasConflicts ? ( } + icon={} + title="This branch has conflicts that must be resolved" + description="Use the command line to resolve conflicts." + /> + ) : isBehind ? ( + } title="This branch is out-of-date with the base branch" description={`Merge the latest changes from ${baseRefName} into this branch.`} action={ @@ -205,54 +202,560 @@ function MergeStatusCard({ owner={owner} repo={repo} pullNumber={pullNumber} - isUpdating={isUpdating} - setIsUpdating={setIsUpdating} + /> + ) : undefined + } + /> + ) : ( + } + title="No conflicts with base branch" + description="Merging can be performed automatically." + action={ + canUpdateBranch ? ( + ) : undefined } /> )} - - ) : ( - - ) - } - title={isMergeBlocked ? "Merging is blocked" : "Ready to merge"} - description={ - isMergeBlocked - ? "All required conditions have not been met." - : "All required conditions have been satisfied." - } - isLast + {/* Merge action footer */} +
); } +// ── Reviews section ───────────────────────────────────────────────── + +function ReviewsSection({ + approvedReviews, + changesRequested, + hasReviewIssue, + owner, + repo, + pullNumber, +}: { + approvedReviews: { + id: number; + state: string; + author: { login: string; avatarUrl: string } | null; + }[]; + changesRequested: { + id: number; + state: string; + author: { login: string; avatarUrl: string } | null; + }[]; + hasReviewIssue: boolean; + owner: string; + repo: string; + pullNumber: number; +}) { + const [open, setOpen] = useState(true); + const queryClient = useQueryClient(); + + const reviewStatus: StatusType = hasReviewIssue + ? "error" + : approvedReviews.length > 0 + ? "success" + : "pending"; + + const title = hasReviewIssue + ? "Changes requested" + : approvedReviews.length > 0 + ? "Changes approved" + : "Review required"; + + const description = hasReviewIssue + ? `${changesRequested.map((r) => r.author?.login).join(", ")} requested changes` + : approvedReviews.length > 0 + ? `${approvedReviews.length} approving review${approvedReviews.length > 1 ? "s" : ""} by reviewers with write access.` + : "Code owner review required by reviewers with write access."; + + const allReviews = [...approvedReviews, ...changesRequested]; + + return ( + + + + + + {allReviews.length > 0 && ( +
+ {allReviews.map((review) => ( +
+ + {review.author && ( + {review.author.login} + )} + + {review.author?.login ?? "Unknown"} + + + {review.state === "APPROVED" + ? "Approved" + : "Changes requested"} + + + + + + + { + void dismissPullReview({ + data: { + owner, + repo, + pullNumber, + reviewId: review.id, + message: "Dismissed via QuickHub", + }, + }) + .then((result) => { + if (result.ok) { + void queryClient.invalidateQueries({ + queryKey: ["github"], + }); + } else { + toast.error(result.error); + checkPermissionWarning( + result, + `${owner}/${repo}`, + ); + } + }) + .catch(() => { + toast.error("Failed to dismiss review"); + }); + }} + > + Dismiss review + + {review.author && ( + { + void requestPullReviewers({ + data: { + owner, + repo, + pullNumber, + reviewers: [review.author!.login], + }, + }) + .then((result) => { + if (result.ok) { + void queryClient.invalidateQueries({ + queryKey: ["github"], + }); + } else { + toast.error(result.error); + checkPermissionWarning( + result, + `${owner}/${repo}`, + ); + } + }) + .catch(() => { + toast.error("Failed to re-request review"); + }); + }} + > + Re-request review + + )} + + +
+ ))} +
+ )} + + + ); +} + +// ── Checks section ────────────────────────────────────────────────── + +function ChecksSection({ + checks, + checkRuns, + allChecksPassed, + hasCheckFailures, +}: { + checks: PullStatus["checks"]; + checkRuns: PullCheckRun[]; + allChecksPassed: boolean; + hasCheckFailures: boolean; +}) { + const [open, setOpen] = useState(true); + + const checkStatus: StatusType = allChecksPassed + ? "success" + : hasCheckFailures + ? "error" + : "pending"; + + const title = allChecksPassed + ? "All checks have passed" + : hasCheckFailures + ? `${checks.failed} failing check${checks.failed > 1 ? "s" : ""}` + : `${checks.pending} pending check${checks.pending > 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.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; + }; + return order(a) - order(b); + }); + + return ( + + + + + +
+ {sortedRuns.map((run) => { + const runStatus = getCheckRunStatus(run); + return ( +
+ + {run.appAvatarUrl && ( + + )} + + {run.name} + + + {runStatus === "success" + ? "Passed" + : runStatus === "failure" + ? "Failed" + : runStatus === "pending" + ? "Pending" + : "Skipped"} + +
+ ); + })} +
+ + + ); +} + +// ── Branch status section ─────────────────────────────────────────── + +function UpdateBranchButton({ + owner, + repo, + pullNumber, +}: { + owner: string; + repo: string; + pullNumber: number; +}) { + const [isUpdating, setIsUpdating] = useState(false); + const queryClient = useQueryClient(); + + const handleUpdate = async () => { + setIsUpdating(true); + try { + const result = await updatePullBranch({ + data: { owner, repo, pullNumber }, + }); + if (result.ok) { + await queryClient.invalidateQueries({ queryKey: ["github"] }); + } else { + toast.error(result.error); + checkPermissionWarning(result, `${owner}/${repo}`); + } + } catch { + toast.error("Failed to update branch"); + } finally { + setIsUpdating(false); + } + }; + + return ( +
+ + + + + + + { + void handleUpdate(); + }} + > +
+ + + Update with merge commit + + + The merge commit will be associated with your account. + +
+ + { + void handleUpdate(); + }} + > +
+ Update with rebase + + This pull request will be rebased on top of the latest changes + and then force pushed. + +
+ + + +
+ ); +} + +// ── Merge footer ──────────────────────────────────────────────────── + +const MERGE_STRATEGIES = [ + { value: "merge" as const, label: "Create a merge commit" }, + { value: "squash" as const, label: "Squash and merge" }, + { value: "rebase" as const, label: "Rebase and merge" }, +]; + +function MergeFooter({ + isMergeBlocked, + owner, + repo, + pullNumber, +}: { + isMergeBlocked: boolean; + owner: string; + repo: string; + pullNumber: number; +}) { + const [mergeMethod, setMergeMethod] = useState<"merge" | "squash" | "rebase">( + "squash", + ); + const [isMerging, setIsMerging] = useState(false); + const queryClient = useQueryClient(); + + const currentStrategy = MERGE_STRATEGIES.find( + (s) => s.value === mergeMethod, + )!; + + const handleMerge = async () => { + setIsMerging(true); + try { + const result = await mergePullRequest({ + data: { owner, repo, pullNumber, mergeMethod }, + }); + if (result.ok) { + await queryClient.invalidateQueries({ queryKey: ["github"] }); + } else { + toast.error(result.error); + checkPermissionWarning(result, `${owner}/${repo}`); + } + } catch { + toast.error("Failed to merge pull request"); + } finally { + setIsMerging(false); + } + }; + + return ( +
+
+
+ + + + + + + {MERGE_STRATEGIES.map((strategy) => ( + setMergeMethod(strategy.value)} + > + + {strategy.value === mergeMethod && ( + + )} + + {strategy.label} + + + + ))} + + +
+
+ {isMergeBlocked && ( +

+ Merging is blocked — all required conditions have not been met. +

+ )} +
+ ); +} + +// ── Shared UI primitives ──────────────────────────────────────────── + function StatusRow({ icon, title, description, action, - isLast, }: { icon: React.ReactNode; title: string; description: string; action?: React.ReactNode; - isLast?: boolean; }) { return ( -
+
{icon}

{title}

@@ -263,14 +766,93 @@ function StatusRow({ ); } -function StatusDot({ color }: { color: string }) { +type StatusType = "success" | "error" | "pending"; + +function StatusIcon({ status }: { status: StatusType }) { + if (status === "success") { + return ( +
+ +
+ ); + } + if (status === "error") { + return ( +
+ +
+ ); + } return ( -
-
+
+
); } +function CheckRunIcon({ + status, +}: { + status: "success" | "failure" | "pending" | "skipped"; +}) { + if (status === "success") { + return ( +
+ +
+ ); + } + if (status === "failure") { + return ( +
+ +
+ ); + } + if (status === "skipped") { + return ( +
+
+
+ ); + } + return ( +
+ +
+ ); +} + +function getCheckRunStatus( + run: PullCheckRun, +): "success" | "failure" | "pending" | "skipped" { + if (run.status !== "completed") return "pending"; + if (run.conclusion === "success" || run.conclusion === "neutral") + return "success"; + if (run.conclusion === "skipped") return "skipped"; + return "failure"; +} + function MergeStatusSkeleton() { return (
@@ -293,51 +875,6 @@ function MergeStatusSkeleton() { ); } -function UpdateBranchButton({ - owner, - repo, - pullNumber, - isUpdating, - setIsUpdating, -}: { - owner: string; - repo: string; - pullNumber: number; - isUpdating: boolean; - setIsUpdating: (value: boolean) => void; -}) { - const queryClient = useQueryClient(); - - const handleUpdate = async () => { - setIsUpdating(true); - try { - const success = await updatePullBranch({ - data: { owner, repo, pullNumber }, - }); - if (success) { - await queryClient.invalidateQueries({ - queryKey: ["github"], - }); - } - } finally { - setIsUpdating(false); - } - }; - - return ( - - ); -} - type TimelineItem = | { type: "comment"; date: string; data: PullComment } | { type: "commit"; date: string; data: PullCommit }; diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx index a1d6f39..ae20de9 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx @@ -205,6 +205,7 @@ function ReviewersSection({ : requestPullReviewers({ data: { owner, repo, pullNumber, reviewers: [login] }, }), + isSuccess: (r) => r.ok, updates: [ { queryKey: pageQueryKey, @@ -247,6 +248,7 @@ function ReviewersSection({ : requestPullReviewers({ data: { owner, repo, pullNumber, teamReviewers: [slug] }, }), + isSuccess: (r) => r.ok, updates: [ { queryKey: pageQueryKey, diff --git a/apps/dashboard/src/lib/github-app.server.ts b/apps/dashboard/src/lib/github-app.server.ts index 1095b5f..d99af71 100644 --- a/apps/dashboard/src/lib/github-app.server.ts +++ b/apps/dashboard/src/lib/github-app.server.ts @@ -79,6 +79,10 @@ export function getGitHubAppAuthConfig() { }; } +export function getGitHubAppSlug(): string | null { + return pickFirstNonEmpty(getWorkerEnv().GITHUB_APP_SLUG) ?? null; +} + export function getGitHubWebhookSecret() { return pickFirstNonEmpty(getWorkerEnv().GITHUB_WEBHOOK_SECRET) ?? null; } diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index ea5dc4f..1966376 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -27,6 +27,7 @@ import type { SubmitReviewInput, UserRepoSummary, } from "./github.types"; +import { getGitHubAppSlug } from "./github-app.server"; import { bustGitHubCache, createGitHubResponseMetadata, @@ -167,6 +168,43 @@ export type PullsFromRepoInput = { direction?: "asc" | "desc"; }; +export type MutationResult = + | { ok: true } + | { ok: false; error: string; installUrl?: string }; + +function toMutationError(action: string, error: unknown): MutationResult { + console.error(`[${action}]`, error); + if (error instanceof RequestError) { + if (error.status === 403) { + const slug = getGitHubAppSlug(); + return { + ok: false, + error: `Failed to ${action}: Insufficient permissions`, + installUrl: slug + ? `https://github.com/apps/${slug}/installations/new` + : undefined, + }; + } + if (error.status === 404) { + return { ok: false, error: `Failed to ${action}: Resource not found` }; + } + if (error.status === 422) { + return { ok: false, error: `Failed to ${action}: Validation failed` }; + } + if (error.status === 409) { + return { + ok: false, + 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` }; +} + export type PullFromRepoInput = { owner: string; repo: string; @@ -810,7 +848,7 @@ async function computePullStatus( const permissions = pull.base.repo.permissions; const canUpdateBranch = - permissions?.push === true || permissions?.admin === true; + !permissions || permissions.push === true || permissions.admin === true; return { reviews: Array.from(latestReviews.values()), @@ -821,6 +859,13 @@ async function computePullStatus( pending, skipped, }, + checkRuns: checkRuns.map((check) => ({ + id: check.id, + name: check.name, + status: check.status, + conclusion: check.conclusion, + appAvatarUrl: check.app?.owner?.avatar_url ?? null, + })), mergeable: pull.mergeable, mergeableState: typeof pull.mergeable_state === "string" ? pull.mergeable_state : null, @@ -1532,10 +1577,10 @@ export const updatePullBody = createServerFn({ method: "POST" }) export const updatePullBranch = createServerFn({ method: "POST" }) .inputValidator(identityValidator) - .handler(async ({ data }): Promise => { + .handler(async ({ data }): Promise => { const context = await getGitHubContext(); if (!context) { - return false; + return { ok: false, error: "Not authenticated" }; } try { @@ -1545,9 +1590,35 @@ export const updatePullBranch = createServerFn({ method: "POST" }) pull_number: data.pullNumber, }); await bustPullDetailCaches(context.session.user.id, data); - return true; - } catch { - return false; + return { ok: true }; + } catch (error) { + return toMutationError("update branch", error); + } + }); + +export type MergePullInput = PullFromRepoInput & { + mergeMethod: "merge" | "squash" | "rebase"; +}; + +export const mergePullRequest = createServerFn({ method: "POST" }) + .inputValidator(identityValidator) + .handler(async ({ data }): Promise => { + const context = await getGitHubContext(); + if (!context) { + return { ok: false, error: "Not authenticated" }; + } + + try { + await context.octokit.rest.pulls.merge({ + owner: data.owner, + repo: data.repo, + pull_number: data.pullNumber, + merge_method: data.mergeMethod, + }); + await bustPullDetailCaches(context.session.user.id, data); + return { ok: true }; + } catch (error) { + return toMutationError("merge pull request", error); } }); @@ -1814,10 +1885,10 @@ export const getOrgTeams = createServerFn({ method: "GET" }) export const requestPullReviewers = createServerFn({ method: "POST" }) .inputValidator(identityValidator) - .handler(async ({ data }): Promise => { + .handler(async ({ data }): Promise => { const context = await getGitHubContext(); if (!context) { - return false; + return { ok: false, error: "Not authenticated" }; } try { @@ -1833,18 +1904,18 @@ export const requestPullReviewers = createServerFn({ method: "POST" }) repo: data.repo, pullNumber: data.pullNumber, }); - return true; - } catch { - return false; + return { ok: true }; + } catch (error) { + return toMutationError("request reviewers", error); } }); export const removeReviewRequest = createServerFn({ method: "POST" }) .inputValidator(identityValidator) - .handler(async ({ data }): Promise => { + .handler(async ({ data }): Promise => { const context = await getGitHubContext(); if (!context) { - return false; + return { ok: false, error: "Not authenticated" }; } try { @@ -1860,9 +1931,44 @@ export const removeReviewRequest = createServerFn({ method: "POST" }) repo: data.repo, pullNumber: data.pullNumber, }); - return true; - } catch { - return false; + return { ok: true }; + } catch (error) { + return toMutationError("remove review request", error); + } + }); + +export type DismissReviewInput = { + owner: string; + repo: string; + pullNumber: number; + reviewId: number; + message: string; +}; + +export const dismissPullReview = createServerFn({ method: "POST" }) + .inputValidator(identityValidator) + .handler(async ({ data }): Promise => { + const context = await getGitHubContext(); + if (!context) { + return { ok: false, error: "Not authenticated" }; + } + + try { + await context.octokit.rest.pulls.dismissReview({ + owner: data.owner, + repo: data.repo, + pull_number: data.pullNumber, + review_id: data.reviewId, + message: data.message, + }); + await bustPullDetailCaches(context.session.user.id, { + owner: data.owner, + repo: data.repo, + pullNumber: data.pullNumber, + }); + return { ok: true }; + } catch (error) { + return toMutationError("dismiss review", error); } }); diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index 071c9bb..29372ec 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -135,6 +135,7 @@ export type PullCheckRun = { name: string; status: string; conclusion: string | null; + appAvatarUrl: string | null; }; export type PullReview = { @@ -152,6 +153,7 @@ export type PullStatus = { pending: number; skipped: number; }; + checkRuns: PullCheckRun[]; mergeable: boolean | null; mergeableState: string | null; behindBy: number | null; diff --git a/apps/dashboard/src/lib/warning-store.ts b/apps/dashboard/src/lib/warning-store.ts new file mode 100644 index 0000000..c8f1779 --- /dev/null +++ b/apps/dashboard/src/lib/warning-store.ts @@ -0,0 +1,73 @@ +import { useSyncExternalStore } from "react"; + +export interface WarningAction { + label: string; + href: string; +} + +export interface Warning { + id: string; + message: string; + dismissible?: boolean; + action?: WarningAction; +} + +let warnings: Warning[] = []; +const listeners = new Set<() => void>(); + +function emitChange() { + for (const listener of listeners) { + listener(); + } +} + +function subscribe(listener: () => void) { + listeners.add(listener); + return () => listeners.delete(listener); +} + +function getSnapshot() { + return warnings; +} + +export function addWarning(warning: Warning) { + if (warnings.some((w) => w.id === warning.id)) return; + warnings = [...warnings, warning]; + emitChange(); +} + +export function removeWarning(id: string) { + warnings = warnings.filter((w) => w.id !== id); + emitChange(); +} + +export function useWarnings() { + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); +} + +/** + * Check a MutationResult for permission errors and surface a warning. + * Call this client-side after a mutation returns. + */ +export function checkPermissionWarning( + result: { ok: boolean; error?: string; installUrl?: string }, + repo: string, +) { + if ( + !result.ok && + result.error && + result.error.includes("Insufficient permissions") + ) { + addWarning({ + id: `permissions:${repo}`, + message: `Your GitHub App may not have sufficient permissions for ${repo}.`, + dismissible: true, + action: result.installUrl + ? { + label: "Configure access", + href: result.installUrl, + } + : undefined, + }); + } +} diff --git a/apps/dashboard/src/routes/__root.tsx b/apps/dashboard/src/routes/__root.tsx index e13ac63..b565914 100644 --- a/apps/dashboard/src/routes/__root.tsx +++ b/apps/dashboard/src/routes/__root.tsx @@ -1,3 +1,4 @@ +import { Toaster } from "@diffkit/ui/components/sonner"; import { TanStackDevtools } from "@tanstack/react-devtools"; import type { QueryClient } from "@tanstack/react-query"; import { @@ -84,6 +85,7 @@ function RootComponent() { return ( + ); } diff --git a/packages/ui/src/components/sonner.tsx b/packages/ui/src/components/sonner.tsx index 0c67f69..b99bbf3 100644 --- a/packages/ui/src/components/sonner.tsx +++ b/packages/ui/src/components/sonner.tsx @@ -27,4 +27,5 @@ const Toaster = ({ ...props }: ToasterProps) => { ); }; +export { toast } from "sonner"; export { Toaster }; From df0924a9d26f5c8dab6c685211a4636a40c22f47 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Thu, 9 Apr 2026 14:59:55 -0400 Subject: [PATCH 2/3] Add admin bypass toggle, merged timeline entry, and improve mutation feedback - Detect admin permissions via repos.get API for reliable bypass check - Show bypass branch protections checkbox when merge is blocked and user is admin - Add merged commit entry to activity timeline with purple merge icon - Map mergeCommitSha and mergedBy fields from GitHub API - Keep buttons in loading state until query revalidation completes - Only show update branch button when branch is actually behind --- .../pulls/detail/pull-detail-activity.tsx | 180 ++++++++++++------ apps/dashboard/src/lib/github.functions.ts | 13 +- apps/dashboard/src/lib/github.types.ts | 3 + 3 files changed, 131 insertions(+), 65 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 6354ac7..e3f582b 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -8,6 +8,7 @@ import { XIcon, } from "@diffkit/icons"; import { Button } from "@diffkit/ui/components/button"; +import { Checkbox } from "@diffkit/ui/components/checkbox"; import { Collapsible, CollapsibleContent, @@ -103,7 +104,11 @@ export function PullDetailActivitySection({

No activity yet.

)} - + {!pr.isMerged && pr.state !== "closed" && (
@@ -147,6 +152,7 @@ function MergeStatusCard({ behindBy, baseRefName, canUpdateBranch, + canBypassProtections, } = status; const approvedReviews = reviews.filter((r) => r.state === "APPROVED"); @@ -211,21 +217,13 @@ function MergeStatusCard({ icon={} title="No conflicts with base branch" description="Merging can be performed automatically." - action={ - canUpdateBranch ? ( - - ) : undefined - } /> )} {/* Merge action footer */} { @@ -570,10 +568,10 @@ function UpdateBranchButton({ } else { toast.error(result.error); checkPermissionWarning(result, `${owner}/${repo}`); + setIsUpdating(false); } } catch { toast.error("Failed to update branch"); - } finally { setIsUpdating(false); } }; @@ -647,11 +645,13 @@ const MERGE_STRATEGIES = [ function MergeFooter({ isMergeBlocked, + canBypassProtections, owner, repo, pullNumber, }: { isMergeBlocked: boolean; + canBypassProtections: boolean; owner: string; repo: string; pullNumber: number; @@ -660,11 +660,12 @@ function MergeFooter({ "squash", ); const [isMerging, setIsMerging] = useState(false); + const [bypassChecks, setBypassChecks] = useState(false); const queryClient = useQueryClient(); - const currentStrategy = MERGE_STRATEGIES.find( - (s) => s.value === mergeMethod, - )!; + const currentStrategy = + MERGE_STRATEGIES.find((s) => s.value === mergeMethod) ?? + MERGE_STRATEGIES[0]; const handleMerge = async () => { setIsMerging(true); @@ -677,65 +678,82 @@ function MergeFooter({ } else { toast.error(result.error); checkPermissionWarning(result, `${owner}/${repo}`); + setIsMerging(false); } } catch { toast.error("Failed to merge pull request"); - } finally { setIsMerging(false); } }; + const isDisabled = (isMergeBlocked && !bypassChecks) || isMerging; + return ( -
-
-
- - - - - - - {MERGE_STRATEGIES.map((strategy) => ( - setMergeMethod(strategy.value)} +
+
+
+
+ + + + + + + {MERGE_STRATEGIES.map((strategy) => ( + setMergeMethod(strategy.value)} + > + + {strategy.value === mergeMethod && ( + + )} + + {strategy.label} + - - - ))} - - + + ))} + + +
+ {isMergeBlocked && !bypassChecks && ( +

+ Merging is blocked — all required conditions have not been met. +

+ )}
- {isMergeBlocked && ( -

- Merging is blocked — all required conditions have not been met. -

+ {isMergeBlocked && canBypassProtections && ( +
+ setBypassChecks(checked === true)} + /> + +
)}
); @@ -882,9 +900,11 @@ type TimelineItem = function ActivityTimeline({ comments, commits, + pr, }: { comments: PullComment[]; commits: PullCommit[]; + pr: PullDetail; }) { const items: TimelineItem[] = [ ...comments.map((comment) => ({ @@ -899,7 +919,7 @@ function ActivityTimeline({ })), ].sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); - if (items.length === 0) return null; + if (items.length === 0 && !pr.isMerged) return null; return (
@@ -980,6 +1000,40 @@ function ActivityTimeline({
); })} + {pr.isMerged && pr.mergedAt && ( +
+
+ +
+ {pr.mergedBy ? ( + {pr.mergedBy.login} + ) : ( +
+ )} + + + {pr.mergedBy?.login ?? "Unknown"} + + {" merged commit "} + {pr.mergeCommitSha && ( + + {pr.mergeCommitSha.slice(0, 7)} + + )} + {" into "} + + {pr.baseRefName} + + + + {formatRelativeTime(pr.mergedAt)} + +
+ )}
); } diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 1966376..be8b1ef 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -383,6 +383,8 @@ function mapPullDetail( headSha: pull.head.sha, baseRefName: pull.base.ref, isMerged: pull.merged, + mergeCommitSha: pull.merge_commit_sha ?? null, + mergedBy: pull.merged_by ? mapActor(pull.merged_by) : null, mergeable: pull.mergeable, mergeableState: typeof pull.mergeable_state === "string" ? pull.mergeable_state : null, @@ -780,7 +782,7 @@ async function computePullStatus( data: PullFromRepoInput, pull: RepoPullDetail, ): Promise { - const [reviewsResponse, checksResponse] = await Promise.all([ + const [reviewsResponse, checksResponse, repoResponse] = await Promise.all([ context.octokit.rest.pulls.listReviews({ owner: data.owner, repo: data.repo, @@ -795,6 +797,9 @@ async function computePullStatus( per_page: 100, }) .catch(() => null), + context.octokit.rest.repos + .get({ owner: data.owner, repo: data.repo }) + .catch(() => null), ]); const latestReviews = new Map< @@ -846,9 +851,11 @@ async function computePullStatus( behindBy = null; } - const permissions = pull.base.repo.permissions; + const permissions = + repoResponse?.data.permissions ?? pull.base.repo.permissions; const canUpdateBranch = !permissions || permissions.push === true || permissions.admin === true; + const canBypassProtections = permissions?.admin === true; return { reviews: Array.from(latestReviews.values()), @@ -872,6 +879,7 @@ async function computePullStatus( behindBy, baseRefName: pull.base.ref, canUpdateBranch, + canBypassProtections, }; } @@ -1598,6 +1606,7 @@ export const updatePullBranch = createServerFn({ method: "POST" }) export type MergePullInput = PullFromRepoInput & { mergeMethod: "merge" | "squash" | "rebase"; + bypassProtections?: boolean; }; export const mergePullRequest = createServerFn({ method: "POST" }) diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index 29372ec..f7557bb 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -65,6 +65,8 @@ export type PullDetail = PullSummary & { headSha: string; baseRefName: string; isMerged: boolean; + mergeCommitSha: string | null; + mergedBy: GitHubActor | null; mergeable: boolean | null; mergeableState?: string | null; requestedReviewers: GitHubActor[]; @@ -159,6 +161,7 @@ export type PullStatus = { behindBy: number | null; baseRefName: string; canUpdateBranch: boolean; + canBypassProtections: boolean; }; export type PullCommit = { From e367e446b14d8275c70c86acb094af904d9cbecb Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Thu, 9 Apr 2026 15:07:01 -0400 Subject: [PATCH 3/3] Add check run output titles, started times, and overflow fix - Show output title next to check run name for completed checks - Show "Started Xs ago" for pending checks - Add overflow-hidden to status card container for clean corners --- .../src/components/pulls/detail/pull-detail-activity.tsx | 9 ++++++++- apps/dashboard/src/lib/github.functions.ts | 2 ++ apps/dashboard/src/lib/github.types.ts | 2 ++ 3 files changed, 12 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 e3f582b..3287835 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -169,7 +169,7 @@ function MergeStatusCard({ const isMergeBlocked = mergeableState === "blocked" || mergeable === false; return ( -
+
{/* Reviews section */} {run.name} + + {runStatus === "pending" && run.startedAt + ? ` — Started ${formatRelativeTime(run.startedAt)}` + : run.outputTitle + ? ` — ${run.outputTitle}` + : null} +