diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index f54c77007c313..b6a917a4a4262 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -149,6 +149,31 @@ it.effect("reads maintainer edits and changes them through the update command", }), ); +it.effect.each([ + { reported: "approved" as const, expected: "approved" }, + { reported: null, expected: null }, + { reported: undefined, expected: undefined }, +])("carries the host's review decision ($reported) on the detail", ({ reported, expected }) => + Effect.gen(function* () { + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" })], + providers: [ + fakePullRequestProvider(SourceControlProviderKind.make("github"), { + getChangeRequest: () => + Effect.succeed({ ...hostedChangeRequest("Body"), reviewDecision: reported }), + }), + ], + }); + const detail = yield* service.detail({ + projectId: "p1" as ProjectId, + repository: "acme/web", + number: 1, + }); + assert.strictEqual(detail.reviewDecision, expected); + assert.strictEqual("reviewDecision" in detail, expected !== undefined); + }), +); + it.effect("refuses maintainer edits on a host without the capability", () => Effect.gen(function* () { let writes = 0; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 9009e2d09e91f..8d50bbd02d8a6 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1883,6 +1883,9 @@ export const make = Effect.gen(function* () { reviewers: changeRequest.reviewers, labels: changeRequest.labels, checks: changeRequest.checks, + ...(changeRequest.reviewDecision === undefined + ? {} + : { reviewDecision: changeRequest.reviewDecision }), mergeCapabilities: changeRequest.mergeCapabilities, viewerPermissions: changeRequest.viewerPermissions, ...(viewer === null || viewer.trim().length === 0 ? {} : { viewer }), diff --git a/apps/web/src/components/pullRequest/PullRequestDetailActions.tsx b/apps/web/src/components/pullRequest/PullRequestDetailActions.tsx index f5a5b4ee77bd9..b8757feb25099 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailActions.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailActions.tsx @@ -4,11 +4,17 @@ import type { PullRequestMergeMethod, PullRequestUpdateMethod, } from "@t3tools/contracts"; -import { CheckIcon, ChevronDownIcon, FolderGit2Icon, GitBranchIcon, PlayIcon } from "lucide-react"; +import { + CheckIcon, + ChevronDownIcon, + CircleCheckIcon, + FolderGit2Icon, + GitBranchIcon, + PlayIcon, +} from "lucide-react"; import type { ReactNode } from "react"; import { AnimatedWidth } from "~/components/AnimatedWidth"; -import { cn } from "~/lib/utils"; import { EnvironmentMachineIcon } from "../EnvironmentMachineIcon"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; @@ -192,6 +198,7 @@ export function PullRequestPrimaryAction({ pullRequestKey, primaryAction, mergeBlockedReason, + readyToMerge, autoMergeArmed, armedAutoMergeLabel, pendingAutoMergeLabel, @@ -205,6 +212,7 @@ export function PullRequestPrimaryAction({ pullRequestKey: string; primaryAction: ReturnType | null; mergeBlockedReason: string | null; + readyToMerge: boolean; autoMergeArmed: boolean; armedAutoMergeLabel: string; pendingAutoMergeLabel: string; @@ -219,6 +227,21 @@ export function PullRequestPrimaryAction({ const actionPending = pendingAction !== null; return ( <> + {readyToMerge ? ( + + + + Ready to merge + + } + /> + + Ready to merge: no conflicts, checks pass, and no review blocks it + + + ) : null} {autoMergeArmed && primaryAction !== "auto-merge-armed" ? ( ) : (primaryAction === "merged" || primaryAction === "closed") && statePresentation !== null ? ( - - - - {statePresentation.label} - + + + {statePresentation.label} ) : null} diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index a4ce4ae7431e0..73e84e8d99115 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -141,6 +141,7 @@ import { readableFailure, resolvePullRequestReferenceHost, resolvePullRequestPrimaryControl, + isPullRequestReadyToMerge, allowedPullRequestMergeMethods, allowsSinglePullRequestMerge, canDeletePullRequestBranch, @@ -1274,6 +1275,19 @@ export function PullRequestDetailPanel({ ? "pending" : latestChecksState; const checksStale = checksState !== detailChecksState; + const readyToMerge = + detail !== null && + isPullRequestReadyToMerge({ + state: detail.state, + isDraft: detail.isDraft, + mergeability: detail.mergeability, + checksState, + reviewDecision: + sharedSummary?.reviewDecision === undefined + ? detail.reviewDecision + : sharedSummary.reviewDecision, + autoMergeEnabled: detail.autoMergeEnabled, + }); const primaryAction = detail ? resolvePullRequestPrimaryControl({ state: detail.state, @@ -1466,6 +1480,7 @@ export function PullRequestDetailPanel({ ) : null} { ); }); + it("is ready to merge only when nothing still waits or blocks", () => { + const ready = { + state: "open" as const, + isDraft: false, + mergeability: "mergeable" as const, + checksState: "passing" as const, + reviewDecision: "approved" as const, + autoMergeEnabled: false, + }; + expect(isPullRequestReadyToMerge(ready)).toBe(true); + expect(isPullRequestReadyToMerge({ ...ready, checksState: null, reviewDecision: null })).toBe( + true, + ); + expect(isPullRequestReadyToMerge({ ...ready, autoMergeEnabled: undefined })).toBe(true); + for (const blocked of [ + { state: "merged" as const }, + { isDraft: true }, + { mergeability: "conflicting" as const }, + { mergeability: "unknown" as const }, + { checksState: "pending" as const }, + { checksState: "failing" as const }, + { reviewDecision: undefined }, + { reviewDecision: "review-required" as const }, + { reviewDecision: "changes-requested" as const }, + { autoMergeEnabled: true }, + ]) { + expect(isPullRequestReadyToMerge({ ...ready, ...blocked })).toBe(false); + } + }); + it("does not offer auto-merge while the host state is unknown", () => { expect( resolvePullRequestPrimaryControl({ diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 97c5919f3ef2e..fab7677145e18 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -31,6 +31,7 @@ import { type PullRequestReaction, type PullRequestMergeMethod, type PullRequestRef, + type PullRequestReviewDecision, type RepositoryIdentity, type PullRequestConversationThread, type PullRequestReviewThread, @@ -118,6 +119,31 @@ export function resolvePullRequestPrimaryControl(input: { return input.canMerge ? "merge" : null; } +/** + * Open, conflict-free, checks green, no blocking review, and auto-merge not armed. + * A `null` review decision means the host requires none; `undefined` means it did not say. + */ +export function isPullRequestReadyToMerge(input: { + readonly state: PullRequestState; + readonly isDraft: boolean; + readonly mergeability: PullRequestMergeability; + readonly checksState: ContractPullRequestChecksState | null; + readonly reviewDecision: PullRequestReviewDecision | null | undefined; + readonly autoMergeEnabled: boolean | undefined; +}): boolean { + return ( + input.state === "open" && + !input.isDraft && + input.mergeability === "mergeable" && + input.checksState !== "pending" && + input.checksState !== "failing" && + input.reviewDecision !== undefined && + input.reviewDecision !== "changes-requested" && + input.reviewDecision !== "review-required" && + input.autoMergeEnabled !== true + ); +} + export function pullRequestCheckoutCommand( provider: SourceControlProviderKind, number: number, diff --git a/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx b/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx index c94403abde4d3..a68f06bfd8aa2 100644 --- a/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx +++ b/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx @@ -87,6 +87,7 @@ it.each(["merge", "enable-auto-merge"] as const)( pullRequestKey="owner/repo/7" primaryAction={primaryAction} mergeBlockedReason={mergeBlockedReason} + readyToMerge={false} autoMergeArmed={false} armedAutoMergeLabel="Auto-merge enabled" pendingAutoMergeLabel="Auto-merge (squash)" diff --git a/apps/web/src/components/ui/badge.tsx b/apps/web/src/components/ui/badge.tsx index ab8f3bbe27174..7c5aeb9511f8a 100644 --- a/apps/web/src/components/ui/badge.tsx +++ b/apps/web/src/components/ui/badge.tsx @@ -26,6 +26,7 @@ const badgeVariants = cva( destructive: "bg-destructive text-white [button&,a&]:hover:bg-destructive/90", error: "bg-destructive/8 text-destructive-foreground dark:bg-destructive/16", info: "bg-info/8 text-info-foreground dark:bg-info/16", + merged: "bg-merged/8 text-merged-foreground dark:bg-merged/16", outline: "border-input bg-background text-foreground dark:bg-input/32 [button&,a&]:hover:bg-accent/50 dark:[button&,a&]:hover:bg-input/48", secondary: "bg-secondary text-secondary-foreground [button&,a&]:hover:bg-secondary/90", diff --git a/apps/web/src/index.css b/apps/web/src/index.css index 654ef108e8e75..6ccc5f1f65f26 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -247,6 +247,8 @@ html[data-mobile-composer-route-transition="true"]::view-transition-old(t3-mobil --color-diff-deletion-foreground: var(--diff-deletion-foreground); --color-info-foreground: var(--info-foreground); --color-info: var(--info); + --color-merged-foreground: var(--merged-foreground); + --color-merged: var(--merged); --color-guest: var(--guest); --color-guest-foreground: var(--guest-foreground); --color-guest-surface: var(--guest-surface); @@ -1155,6 +1157,8 @@ html[data-mobile-composer-route-transition="true"]::view-transition-old(t3-mobil --destructive-foreground: var(--error-foreground); --info: var(--color-blue-500); --info-foreground: var(--color-blue-700); + --merged: var(--color-violet-500); + --merged-foreground: var(--color-violet-700); /* Messages from a guest link, and the sidebar Request status. */ --guest: var(--color-fuchsia-500); --guest-foreground: var(--color-fuchsia-600); @@ -1212,6 +1216,7 @@ html[data-mobile-composer-route-transition="true"]::view-transition-old(t3-mobil --border: --alpha(var(--color-white) / 8%); --input: --alpha(var(--color-white) / 11%); --info-foreground: var(--color-blue-400); + --merged-foreground: var(--color-violet-400); --guest-foreground: var(--color-fuchsia-300); --success-foreground: var(--color-emerald-400); --warning-foreground: var(--color-amber-400); diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 014b26272af73..f23a2864d829c 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -650,6 +650,8 @@ export const PullRequestDetail = Schema.Struct({ reviewers: Schema.Array(PullRequestActor), labels: Schema.Array(PullRequestLabel), checks: Schema.Array(PullRequestCheck), + /** Null means the host requires no review; absent means the read did not report one. */ + reviewDecision: Schema.optional(Schema.NullOr(PullRequestReviewDecision)), mergeCapabilities: PullRequestMergeCapabilities, viewer: Schema.optional(TrimmedNonEmptyString), baseComparison: Schema.optional(PullRequestBaseComparison),