fix(web): don't offer merge when GitHub blocks it

apps/hub/src/github/github-read-access.postgres.test.ts

@@ -49,6 +49,11 @@ const READS: Record<string, unknown> = {
   [`/repos/${REPO}/pulls/8/commits`]: [{ sha: HEAD, commit: { message: "fix: the widget" } }],
   [`/repos/${REPO}/pulls/8/files`]: [{ filename: "src/widget.ts", status: "modified", additions: 1, deletions: 0, patch: "@@ -1 +1 @@\n+fixed" }],
   [`/repos/${REPO}/issues/8/comments`]: [],
+  [`/repos/${REPO}/pulls/9`]: { number: 9, title: "Held by a ruleset", state: "open", user: { login: "octocat" }, head: { sha: HEAD, ref: "held" }, mergeable: true, mergeable_state: "blocked", labels: [], assignees: [] },
+  [`/repos/${REPO}/pulls/9/commits`]: [],
+  [`/repos/${REPO}/pulls/9/files`]: [],
+  [`/repos/${REPO}/pulls/9/reviews`]: [],
+  [`/repos/${REPO}/issues/9/comments`]: [],
   [`/repos/${REPO}/commits/${HEAD}/check-runs`]: { check_runs: [] },
   [`/repos/${REPO}/commits/${HEAD}/status`]: { statuses: [] },
   "/orgs/acme/teams/secret/memberships/octocat": { state: "active" },
@@ -61,6 +66,7 @@ async function github(req: Request): Promise<Response> {
   if (req.method === "GET" && pathname in READS) return Response.json(READS[pathname]);
   if (pathname === `/repos/${REPO}/installation`) return Response.json({ id: 1 });
   if (pathname === "/app") return Response.json({ slug: "triage" });
+  if (req.method === "POST" && pathname === "/graphql") return Response.json({ data: { repository: null } });
   if (req.method === "GET" && pathname === `/repos/${REPO}/pulls/8/reviews`) {
     return Response.json(sentReviews.map((review, index) => ({ id: 100 + index, user: { login: "triage[bot]" }, state: "COMMENTED", commit_id: review.commit_id, body: review.body })));
   }
@@ -186,6 +192,20 @@ test("PostgreSQL: holding read or use on the GitHub credential reads pull reques
   expect(requested.some((line) => line.startsWith("PUT"))).toBe(false);
 }, 60_000);
 
+test("PostgreSQL: pull request details ask GitHub for the review decision only when the merge is blocked", async () => {
+  requested.length = 0;
+  await prDetails(new Request(`http://hub.test/pull?repo=${REPO}&number=8`), member("usr_member"));
+  expect(requested).not.toContain("POST /graphql");
+  await prDetails(new Request(`http://hub.test/pull?repo=${REPO}&number=9`), member("usr_member"));
+  expect(requested).toContain("POST /graphql");
+});
+
+test("PostgreSQL: a review decision GitHub does not give leaves it unknown and still serves the details", async () => {
+  const blocked = await prDetails(new Request(`http://hub.test/pull?repo=${REPO}&number=9`), member("usr_member"));
+  expect(blocked.status).toBe(200);
+  expect(await blocked.json()).toMatchObject({ pr: { number: 9, mergeableState: "blocked", reviewDecision: null } });
+});
+
 test("PostgreSQL: pull request details serve only repositories connected to the workspace", async () => {
   const res = await prDetails(new Request("http://hub.test/pull?repo=acme/other&number=8"), member("usr_member"));
   expect(res.status).toBe(409);

apps/hub/src/github/github-refusal.test.ts

@@ -0,0 +1,9 @@
+import { expect, test } from "bun:test";
+import { githubFailed } from "./github-refusal.js";
+
+test("a refusal keeps the status and GitHub's reason, never the method or path", async () => {
+  const err = new Error("github PUT /repos/acme/widgets/pulls/8/merge -> 405 At least 1 approving review is required by reviewers with write access.");
+  const res = githubFailed("merge acme/widgets#8", err);
+  expect(res.status).toBe(502);
+  expect(await res.json()).toEqual({ error: { code: "github_failed", message: "GitHub refused to merge acme/widgets#8 (HTTP 405): At least 1 approving review is required by reviewers with write access." } });
+});

apps/hub/src/github/github-refusal.ts

@@ -16,11 +16,13 @@ export const GITHUB_VERB = {
   close: "close",
 } as const;
 
-/** A host's failure as a sentence; the transport line (method, path, status) is for logs only. */
+/** A host's failure as a sentence with the host's own reason; the method and path are for logs only. */
 function refusal(host: string, what: string, err: unknown): string {
   const message = errorText(err);
-  const status = /(?:-> |answered )(\d{3})\b/.exec(message)?.[1];
-  return `${host} refused to ${what}${status ? ` (HTTP ${status})` : `: ${message}`}.`;
+  const transport = /-> (\d{3})\b\s*(.*)$/s.exec(message);
+  if (!transport) return `${host} refused to ${what}: ${message.replace(/\.$/, "")}.`;
+  const [, status, reason] = transport;
+  return `${host} refused to ${what} (HTTP ${status})${reason ? `: ${reason.replace(/\.$/, "")}` : ""}.`;
 }
 
 export function githubFailed(what: string, err: unknown): Response {

apps/hub/src/github/pr-details.ts

@@ -6,7 +6,7 @@ import { eq } from "drizzle-orm";
 import { type } from "arktype";
 import { schema } from "@intx/db";
 import * as bitbucket from "@corbits/bitbucket-datacenter-reads";
-import { appLogin, getChecks, getPr, getReviews, listIssueComments, listPrCommits, listPrFiles, listReferencedIssues, type GithubFetch } from "@corbits/github-reads";
+import { appLogin, getChecks, getPr, getReviews, listIssueComments, listPrCommits, listPrFiles, listReferencedIssues, readReviewDecision, type GithubFetch, type ReviewDecision } from "@corbits/github-reads";
 import { errorText } from "@corbits/common";
 import { failure, sourceCredentialGrant } from "./portal-credential.js";
 import type { Member } from "../integrations/app.js";
@@ -17,22 +17,35 @@ export const GITHUB_PR_DETAILS_PATH = "/api/integrations/github-pull";
 
 const Query = type({ repo: /^[\w.-]+\/[\w.-]+$/, number: "string.integer.parse" }).pipe((q, ctx) => (q.number > 0 ? q : ctx.error("number must be positive")));
 
-async function githubDetails(gh: GithubFetch, repo: string, number: number) {
+type Log = (entry: Record<string, unknown>) => void;
+
+/** Only a blocked merge asks whether a review is what blocks it; a decision GitHub will not give leaves the block unexplained, not the details unread. */
+async function reviewDecisionOf(gh: GithubFetch, repo: string, number: number, log: Log): Promise<ReviewDecision | null> {
+  try {
+    return await readReviewDecision(gh, repo, number);
+  } catch (err) {
+    log({ level: "warn", msg: "review_decision_failed", repo, number, error: errorText(err) });
+    return null;
+  }
+}
+
+async function githubDetails(gh: GithubFetch, repo: string, number: number, log: Log) {
   const [pr, files, commits, comments] = await Promise.all([
     getPr(gh, repo, number),
     listPrFiles(gh, repo, number),
     listPrCommits(gh, repo, number),
     listIssueComments(gh, repo, number),
   ]);
   const text = [pr.title ?? "", pr.body ?? "", ...commits.map((c) => c.message)].join("\n");
-  const [issues, checks, reviews] = await Promise.all([
+  const [issues, checks, reviews, reviewDecision] = await Promise.all([
     listReferencedIssues(gh, repo, number, text),
     getChecks(gh, repo, pr.sha),
     getReviews(gh, repo, number, function login() {
       return appLogin(gh);
     }),
+    pr.mergeableState === "blocked" ? reviewDecisionOf(gh, repo, number, log) : null,
   ]);
-  return { pr, files, commits, comments, issues, checks, reviews };
+  return { pr: { ...pr, reviewDecision }, files, commits, comments, issues, checks, reviews };
 }
 
 /** Data Center lists changes without patches or line counts and has no issue tracker of its own. */
@@ -48,13 +61,13 @@ async function bitbucketDetails(bb: bitbucket.BitbucketFetch, repo: string, numb
   return { pr, files: files.map((file) => ({ ...file, additions: 0, deletions: 0 })), commits, comments, issues: [], checks, reviews };
 }
 
-export function createGithubPrDetails(deps: SourceFetchDeps & { log: (entry: Record<string, unknown>) => void }) {
+export function createGithubPrDetails(deps: SourceFetchDeps & { log: Log }) {
   const sourceFetch = createSourceFetch(deps);
 
   async function detailsOf(tenantId: string, principalId: string, record: RepoRecord, number: number) {
     const source = await sourceFetch(tenantId, principalId, record);
     if (!source) throw new Error(`no readable ${sourceOf(record)} credential`);
-    return "bb" in source ? bitbucketDetails(source.bb, record.name, number) : githubDetails(source.gh, record.name, number);
+    return "bb" in source ? bitbucketDetails(source.bb, record.name, number) : githubDetails(source.gh, record.name, number, deps.log);
   }
 
   return async function handle(req: Request, { tenantId, principalId }: Member): Promise<Response> {

apps/web/src/lib/hub-api.ts

@@ -1073,6 +1073,14 @@ export type GithubPullDetail = {
     sha: string;
     base: string;
     mergeable: boolean | null;
+    /** GitHub's `mergeable_state`, such as `blocked` when the branch's rules refuse the merge; absent on Bitbucket. */
+    mergeableState?: string;
+    /** Read only when GitHub blocks the merge; null when no review is required or the read failed. */
+    reviewDecision?: "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | null;
+    /** Whether Bitbucket's merge checks allow the merge; absent on GitHub. */
+    canMerge?: boolean;
+    /** Why Bitbucket's merge checks refuse the merge, one summary per check; absent on GitHub. */
+    mergeVetoes?: string[];
     requestedReviewers: number;
     additions: number;
     deletions: number;

apps/web/src/lib/inbox-pane.ts

@@ -11,15 +11,32 @@ export function titleText(title: string | null): string {
   return title === null ? UNTITLED : title;
 }
 
-type PaneFacts = { title: string | null; author: string | null; draft: boolean | null; mergeable: boolean | null };
+/** Why the host holds a merge free of conflicts. */
+export type HeldBlock = "behind" | "review-required" | "changes-requested" | "blocked";
+
+type HostBlock = "mergeability-unknown" | HeldBlock;
+
+/** `hostBlock` is known only once the host's detail has loaded. */
+type PaneFacts = { title: string | null; author: string | null; draft: boolean | null; mergeable: boolean | null; hostBlock: HostBlock | null };
+
+const GITHUB_BLOCK: Partial<Record<string, HostBlock>> = { unknown: "mergeability-unknown", behind: "behind", blocked: "blocked" };
+
+const REVIEW_BLOCK: Partial<Record<string, HostBlock>> = { REVIEW_REQUIRED: "review-required", CHANGES_REQUESTED: "changes-requested" };
+
+function hostBlock(pr: GithubPullDetail["pr"]): HostBlock | null {
+  if (pr.canMerge === false) return "blocked";
+  const block = pr.mergeableState === undefined ? undefined : GITHUB_BLOCK[pr.mergeableState];
+  if (block === "blocked" && pr.reviewDecision) return REVIEW_BLOCK[pr.reviewDecision] ?? block;
+  return block ?? null;
+}
 
 /** Once GitHub's detail has loaded it replaces the verdict's facts outright, so its null mergeable means GitHub is still computing it. */
 export function paneFacts(item: PrItem, detail: GithubPullDetail | undefined): PaneFacts {
-  if (detail === undefined) return { title: item.title, author: item.author, draft: item.draft, mergeable: item.mergeable };
-  return { title: detail.pr.title, author: detail.pr.author, draft: detail.pr.draft, mergeable: detail.pr.mergeable };
+  if (detail === undefined) return { title: item.title, author: item.author, draft: item.draft, mergeable: item.mergeable, hostBlock: null };
+  return { title: detail.pr.title, author: detail.pr.author, draft: detail.pr.draft, mergeable: detail.pr.mergeable, hostBlock: hostBlock(detail.pr) };
 }
 
-export type ActionBlocker = "no-number" | "running" | "read-only" | "busy" | "unchecked-commits" | "mergeability-unknown" | "not-mergeable" | "draft";
+export type ActionBlocker = "no-number" | "running" | "read-only" | "busy" | "unchecked-commits" | "not-mergeable" | "draft" | HostBlock;
 
 export function hasUncheckedCommits(item: Pick<PrItem, "verdictSha" | "headSha" | "waitingHeadSha">): boolean {
   const head = item.waitingHeadSha ?? item.headSha;
@@ -45,7 +62,7 @@ function actionBlocker(kind: PaneKind, gate: PaneGate): ActionBlocker | null {
   if (gate.facts.mergeable === null) return "mergeability-unknown";
   if (!gate.facts.mergeable) return "not-mergeable";
   if (gate.facts.draft === true) return "draft";
-  return null;
+  return gate.facts.hostBlock;
 }
 
 type PaneAction<K extends PaneKind = PaneKind> = { kind: K; blocker: ActionBlocker | null };

apps/web/src/lib/pane-report.test.ts

@@ -195,3 +195,21 @@ describe("passedChecks", () => {
     expect(passedChecks([check({ check: "a" }), check({ check: "b", gate: true }), check({ check: "c", gate: true, result: "fail" })]).map((row) => row.check)).toEqual(["a", "b"]);
   });
 });
+
+describe("a merge the host blocks", () => {
+  const ready = { state: "ready", verdict: "pass", ci: "success" } as const;
+
+  test("a pass free of conflicts that a ruleset holds for a review offers no merge and says why", () => {
+    const live = detail({ mergeable: true, mergeableState: "blocked", reviewDecision: "REVIEW_REQUIRED" });
+    expect(barOf(item(ready), live).primary?.blocker).toBe("review-required");
+    expect(statusOf(ready, { detail: live })).toMatchObject({ tone: "you", text: "Ready, but waiting for a required review" });
+  });
+
+  test("other blocks on GitHub and Bitbucket's merge checks hold the merge too", () => {
+    expect(statusOf(ready, { detail: detail({ mergeableState: "blocked", reviewDecision: null }) }).text).toBe("Ready, but the branch's rules block the merge");
+    expect(statusOf(ready, { detail: detail({ mergeableState: "behind" }) }).text).toBe("Ready, but behind its base branch");
+    expect(statusOf(ready, { detail: detail({ mergeableState: "unknown" }) }).text).toBe("Ready, once GitHub has checked for conflicts");
+    expect(statusOf(ready, { detail: detail({ canMerge: false, mergeVetoes: ["Needs 2 approvals"] }) }).text).toBe("Ready, but the merge checks say: Needs 2 approvals");
+    expect(statusOf(ready, { detail: detail({ mergeableState: "clean" }) }).text).toBe("Ready to merge");
+  });
+});

apps/web/src/lib/pane-report.ts

@@ -1,6 +1,6 @@
 import { citationOf } from "@corbits/triage-contracts";
 import type { CheckResult, GithubPullDetail, PrItem, QueueState, RunFailure } from "./hub-api.ts";
-import { currentHead, hasUncheckedCommits, paneActions, type ActionBlocker, type MenuEntry, type PaneGate, type PaneKind } from "./inbox-pane.ts";
+import { currentHead, hasUncheckedCommits, paneActions, type ActionBlocker, type HeldBlock, type MenuEntry, type PaneGate, type PaneKind } from "./inbox-pane.ts";
 import { inboxAction, inboxStatus, isPostedToAuthor, isReReview, postedLabel, primaryLabel, waitsOn, type HandledMark } from "./inbox-view.ts";
 import { upperFirst } from "./triage-view.ts";
 
@@ -53,6 +53,24 @@ const NOT_YET: Partial<Record<ActionBlocker, string>> = {
   "mergeability-unknown": "Ready, once GitHub has checked for conflicts",
 };
 
+const HELD_REASON: Record<HeldBlock, string> = {
+  behind: "behind its base branch",
+  "review-required": "waiting for a required review",
+  "changes-requested": "changes are requested",
+  blocked: "the branch's rules block the merge",
+};
+
+export function isHeldBlock(blocker: ActionBlocker | null): blocker is HeldBlock {
+  return blocker !== null && blocker in HELD_REASON;
+}
+
+/** Why the host holds the merge; Bitbucket's merge checks name their own reasons. */
+export function heldReason(block: HeldBlock, detail: GithubPullDetail | undefined): string {
+  const vetoes = detail?.pr.mergeVetoes ?? [];
+  if (block === "blocked" && vetoes.length) return `the merge checks say: ${vetoes.join("; ")}`;
+  return HELD_REASON[block];
+}
+
 function things(count: number): string {
   return `${count} ${count === 1 ? "thing" : "things"}`;
 }
@@ -136,7 +154,9 @@ export function paneStatus({ item, detail, handled, approved, primary }: StatusI
   if (unsure > 0) return status("you", `Not sure about ${things(check)} — your call`);
   if (check > 0) return status("you", `Couldn't check ${things(check)}: ${problems.filter((problem) => problem.severity === "cant").map((problem) => problem.text ?? problem.check.check).join("; ")}`);
   if (item.state !== "ready") return status("you", inboxStatus(item));
-  const blocked = primary?.kind === "merge" && primary.blocker !== null ? NOT_YET[primary.blocker] : undefined;
+  const blocker = primary?.kind === "merge" ? primary.blocker : null;
+  if (isHeldBlock(blocker)) return status("you", `Ready, but ${heldReason(blocker, detail)}`);
+  const blocked = blocker === null ? undefined : NOT_YET[blocker];
   if (blocked !== undefined) return status("you", blocked);
   return status("ready", approved ? "Approved by Triage · ready for you to merge" : "Ready to merge");
 }

apps/web/src/pages/PRDetail.tsx

@@ -11,7 +11,8 @@ import { useApprovals } from "../lib/tenant-entities.ts";
 import { QUEUE_STATE_LABEL, type CheckResult, type GithubPullDetail, type PrItem } from "../lib/hub-api.ts";
 import { useGithubPull } from "../lib/github-pull.ts";
 import { errorText } from "@corbits/common";
-import { approvedHead, checkedHead, hasUncheckedCommits, paneFacts, titleText } from "../lib/inbox-pane.ts";
+import { approvedHead, canRun, checkedHead, hasUncheckedCommits, paneFacts, titleText } from "../lib/inbox-pane.ts";
+import { heldReason, isHeldBlock } from "../lib/pane-report.ts";
 import { offersClose, pendingDos, type PendingDo } from "../lib/pending-dos.ts";
 import { useApproveDo, useHeldDoKeys } from "../lib/held-dos.ts";
 import { ApprovalCard } from "../components/ApprovalCard.tsx";
@@ -30,6 +31,7 @@ import {
   checkValueText,
   findPrItem,
   relativeTime,
+  upperFirst,
   verdictText,
 } from "../lib/triage-view.ts";
 
@@ -320,10 +322,9 @@ export default function PRDetail() {
   const selected = useMemo(() => files.find((file) => file.path === filePath) ?? files[0], [files, filePath]);
   const pending = approval?.status.toLowerCase() === "pending";
   const gated = Boolean(item?.needsHuman && pending);
-  const facts = item === undefined ? null : paneFacts(item, pull.data);
-  const mergeable = facts === null ? null : facts.mergeable;
+  const gate = item === undefined ? null : { item, facts: paneFacts(item, pull.data), readOnly, busy };
   const closed = pull.data?.pr.state === "closed";
-  const canMerge = facts !== null && facts.mergeable === true && facts.draft !== true;
+  const canMerge = gate !== null && canRun("merge", gate);
   const [handled, setHandled] = useState<Record<string, "posted" | "dismissed">>({});
   const head = pull.data?.pr.sha ?? item?.sha ?? "";
   const handledKey = `${item?.key}@${head}`;
@@ -476,8 +477,10 @@ export default function PRDetail() {
 
   function mergeBadge(): ReactNode {
     if (closed) return <Badge>{pull.data?.pr.merged ? "Merged" : "Closed"}</Badge>;
-    if (mergeable === null) return null;
-    return <Badge>{mergeable ? "Mergeable" : "Not mergeable"}</Badge>;
+    if (gate === null || gate.facts.mergeable === null || gate.facts.hostBlock === "mergeability-unknown") return null;
+    if (!gate.facts.mergeable) return <Badge>Not mergeable</Badge>;
+    if (isHeldBlock(gate.facts.hostBlock)) return <Badge>{`Merge blocked · ${upperFirst(heldReason(gate.facts.hostBlock, pull.data))}`}</Badge>;
+    return <Badge>Mergeable</Badge>;
   }
 
   function sliverText(what: string, loaded: string): string {
@@ -566,7 +569,7 @@ export default function PRDetail() {
               {!closed && (
                 <>
                   <Button type="button" variant="outline" disabled={!item.number || busy || readOnly} onClick={() => void requestWrite("comment")}>Comment</Button>
-                  <Button type="button" variant={canMerge && !gated ? "primary" : "outline"} disabled={!item.number || !canMerge || busy || readOnly} onClick={() => void requestWrite("merge")}>Merge</Button>
+                  <Button type="button" variant={canMerge && !gated ? "primary" : "outline"} disabled={!canMerge} onClick={() => void requestWrite("merge")}>Merge</Button>
                   <Menu>
                     <MenuTrigger asChild><Button type="button" variant="outline" aria-label="More actions">More</Button></MenuTrigger>
                     <MenuContent align="end">