diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index fb36fcd08752..9ecfafd7a517 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -150,6 +150,7 @@ import { pullRequestCheckoutCommand, pullRequestFindingKey, pullRequestHandoffLabels, + mergeRefusalHint, PULL_REQUEST_MERGE_METHOD_LABELS, readableFailure, readPullRequestDetailSnapshot, @@ -988,7 +989,13 @@ export function PullRequestDetailPanel({ const hint = updateMethod === "rebase" ? UPDATE_BRANCH_REBASE_FAILURE_HINT - : ACTION_FAILURE_HINTS[action]; + : action === "merge" && detail !== null + ? (mergeRefusalHint({ + mergeability: detail.mergeability, + reviewDecision: sharedSummary?.reviewDecision, + checksState, + }) ?? ACTION_FAILURE_HINTS.merge) + : ACTION_FAILURE_HINTS[action]; toastManager.add({ type: "error", title: ACTION_FAILURE_LABELS[action], diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index ac0c9d01d17e..478c01acfd53 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -40,6 +40,7 @@ import { pullRequestCheckoutCommand, pullRequestFindingKey, pullRequestReviewOutcome, + mergeRefusalHint, readableFailure, readPullRequestDetailSnapshot, resolveDisplayedPullRequestDetail, @@ -1045,6 +1046,47 @@ describe("what to say when an action fails", () => { }); }); +describe("why a merge was refused", () => { + const clear = { + mergeability: "mergeable", + reviewDecision: null, + checksState: "passing", + } as const; + + it("names a required review, the blocker the host only calls a policy", () => { + expect(mergeRefusalHint({ ...clear, reviewDecision: "review-required" })).toContain( + "approving review", + ); + }); + + it("puts conflicts ahead of reviews and checks, since they need fixing first", () => { + expect( + mergeRefusalHint({ + mergeability: "conflicting", + reviewDecision: "review-required", + checksState: "failing", + }), + ).toContain("conflicts"); + }); + + it("names failing or running checks when reviews are not in the way", () => { + expect(mergeRefusalHint({ ...clear, checksState: "failing" })).toContain("failing"); + expect(mergeRefusalHint({ ...clear, checksState: "pending" })).toContain("still running"); + }); + + it("blames the branch rules when nothing visible explains the refusal", () => { + // The author's own approval shows as "approved" but satisfies no rule. + expect(mergeRefusalHint({ ...clear, reviewDecision: "approved" })).toContain("branch rules"); + expect(mergeRefusalHint(clear)).toContain("branch rules"); + }); + + it("leaves the generic hint when the host has not said whether it can merge", () => { + expect( + mergeRefusalHint({ mergeability: "unknown", reviewDecision: undefined, checksState: null }), + ).toBeNull(); + }); +}); + describe("findings that are already on a line", () => { it("does not quote a resolved thread's comment as a remark with nowhere to hang", () => { const resolved: PullRequestReviewThread = { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 2ec4863072dc..5cabc09e1ff1 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -17,6 +17,7 @@ import { type PullRequestReaction, type PullRequestRef, type RepositoryIdentity, + type PullRequestReviewDecision, type PullRequestReviewThread, type PullRequestState, type PullRequestUpdateMethod, @@ -1075,6 +1076,43 @@ export function readableFailure(failure: unknown, hint: string): string { return bounded; } +/** + * What to say under a refused merge when the host gave no reason. `gh` reports a blocked merge + * only as "the base branch policy prohibits the merge", so this names the blocker from what the + * page already knows. Null when mergeability is unknown, which leaves the generic hint. + */ +export function mergeRefusalHint(state: { + readonly mergeability: PullRequestMergeability; + readonly reviewDecision: PullRequestReviewDecision | null | undefined; + readonly checksState: PullRequestChecksState | null; +}): string | null { + if (state.mergeability === "unknown") return null; + if (state.mergeability === "conflicting") { + return "The branch conflicts with the base. Resolve the conflicts, then merge again."; + } + // GitHub only reports a missing review when a branch rule requires one. The reverse does not + // hold: the server shows "approved" once any approval exists, including the author's own, which + // no rule counts. + if (state.reviewDecision === "review-required") { + return "The branch rules require an approving review before this can merge."; + } + if (state.reviewDecision === "changes-requested") { + return "A reviewer requested changes. Their review has to be addressed before this can merge."; + } + if (state.checksState === "failing") { + return "Some checks are failing, and the branch rules may require them to pass."; + } + if (state.checksState === "pending") { + return "Some checks are still running or awaiting action, and the branch rules may require them to pass."; + } + // Mergeable with nothing failing, yet refused: a branch rule the page cannot see, such as an + // approval from someone other than the author, signed commits, or resolved conversations. + if (state.mergeability === "mergeable") { + return "The branch rules blocked the merge. They may need an approval from someone other than the author, signed commits, or resolved conversations. The pull request on the host lists what is missing."; + } + return null; +} + /** * Where the branch stands against its base, said the way GitHub says it: current, out of date but * still cleanly mergeable, or conflicting. Only the middle one is an offer — the conflicts row