feat(web): surface PR merge-conflict & mergeability state in the CI popover (#1448)

* feat(web): surface PR mergeability + conflict state in the CI popover The CI hover popover showed checks/reviews but nothing about mergeability, so an unmergeable PR silently dropped its merge button with no reason. Add a mergeability row that reuses the detail panel's conflict banner + "Resolve conflicts" agent prompt for dirty PRs, a "Behind base" chip, and a branch-protection note explaining why a blocked PR is gated. Replace the compact merge split-button+dropdown with an inline primary + "or <method>" buttons so the menu's detached portal no longer closes the hover popover. * feat(web): reflect PR mergeability in status colors and badges The PR status pill, badge, and chat-input CI chip derived their colour from checks/reviews only, so a conflicted or blocked PR still read as a green "all good" check. Map dirty -> red, behind -> amber, and branch-protection blocked -> amber across getPRStatusColor, the PRStatusIcon badge, and the chip glyph, so the indicators agree with the popover. Blocked stays sky/in-progress while a requested review is still outstanding (that's the awaiting-review gate, not a hard block). * refactor(web): remove dead merge-menu keep-open plumbing The compact merge control was redesigned to inline buttons (no dropdown portal), which already fixes the hover popover closing. That leaves the keepOpen ref / setKeepOpen / onMergeMenuOpenChange / onMenuOpenChange chain threaded through usePopoverInteractions, PRCIPopover, and PRMergeButton unreachable: the popover always renders PRMergeButton with compact, and the only DropdownMenu lives in the non-compact detail-panel path that never passes the callback. Drop the unused scaffolding (flagged by OpenCode + greptile). * fix(web): keep CI popover quiet for blocked PRs only awaiting review PRMergeabilityRow rendered "Blocked by branch protection" for any blocked PR, including one blocked solely because a requested reviewer hasn't responded. That contradicted the chip/pill/badge, which deliberately read that case as the calm "awaiting review" state. Skip the note (render nothing) when isPRAwaitingReview is true, and narrow blockedReason so an empty checks_state no longer falsely claims a required status check failed. Flagged by Claude (blocker) and greptile. * refactor(web): rename mergeability-notice compact prop to popover The `compact` prop made the chip/text larger (to match the popover's text-xs rows) while ConflictBanner's `compact` tightened padding — so "compact" rendered bigger, which reads as inverted. Rename it to `popover` (the variant rendered inside the CI hover popover) and document that the chip intentionally matches the popover rows. No visual change. Flagged by cubic. * fix(web): show default cursor on disabled compact merge alternates The compact "or <method>" alternates kept cursor-pointer while disabled (during the ~1s merging state); add disabled:cursor-default to match the primary button. Flagged by OpenCode.
2026-06-19 14:56:24 +01:00
parent 2a890c7975
commit 8e55c3c477
10 changed files with 577 additions and 40 deletions
@@ -27,6 +27,7 @@ import {
 } from "@/lib/github/check-buckets";
 import type { CheckRun, TaskPR } from "@/lib/types/github";
 import { PRMergeButton } from "./pr-merge-button";
+import { PRMergeabilityRow } from "./pr-mergeability-row";

 type CountsView = {
  passed: number;
@@ -543,6 +544,7 @@ export function PRCIPopover({
            <PRReviewRow pr={pr} />
            <PRCommentsRow pr={pr} />
          </div>
+          <PRMergeabilityRow pr={pr} />
          <PRMergeButton taskPR={pr} onMerged={refetch} compact />
        </>
      )}
@@ -108,19 +108,50 @@ function MergeButtonShell({
  extraMethods: MergeMethod[];
  onPickMethod: (m: MergeMethod) => void;
 }) {
+  // Compact (hover popover): a single full-width primary button plus quiet
+  // "or <method>" links for the alternates. Deliberately avoids a dropdown —
+  // its menu renders in a detached portal that closes the hover popover the
+  // moment the pointer leaves it (the bug this replaces). Plain buttons keep
+  // every action inside the popover's own DOM.
+  if (compact) {
+    return (
+      <div className="flex flex-col gap-1.5">
+        <button
+          type="button"
+          data-testid="pr-merge-button"
+          onClick={onPrimaryClick}
+          disabled={disabled}
+          className="inline-flex w-full items-center justify-center gap-1.5 rounded-md bg-green-600 px-2 py-1.5 text-xs font-medium text-white shadow-sm transition-colors hover:bg-green-700 disabled:cursor-default disabled:opacity-60 cursor-pointer"
+        >
+          <IconGitMerge className="h-3.5 w-3.5" />
+          {label}
+        </button>
+        {extraMethods.length > 0 && (
+          <div className="flex items-center justify-center gap-1 text-[11px] text-muted-foreground">
+            <span>or</span>
+            {extraMethods.map((m) => (
+              <button
+                key={m}
+                type="button"
+                data-testid={`pr-merge-method-${m}`}
+                disabled={disabled}
+                onClick={(e) => {
+                  e.stopPropagation();
+                  onPickMethod(m);
+                }}
+                className="cursor-pointer rounded px-1.5 py-0.5 font-medium hover:bg-muted hover:text-foreground disabled:cursor-default disabled:opacity-60"
+              >
+                {mergeShortLabel(m)}
+              </button>
+            ))}
+          </div>
+        )}
+      </div>
+    );
+  }
+
  const showDropdown = extraMethods.length > 0;
-  const primaryBtn = compact ? (
-    <button
-      type="button"
-      data-testid="pr-merge-button"
-      onClick={onPrimaryClick}
-      disabled={disabled}
-      className={`inline-flex items-center gap-1 bg-green-600 px-2 py-0.5 text-[11px] font-medium text-white hover:bg-green-700 dark:bg-green-600 dark:hover:bg-green-500 disabled:opacity-60 cursor-pointer ${showDropdown ? "rounded-l-full" : "rounded-full"}`}
-    >
-      <IconGitMerge className="h-3 w-3" />
-      {label}
-    </button>
-  ) : (
+  const primaryBtn = (
    <Button
      data-testid="pr-merge-button"
      size="sm"
@@ -134,11 +165,11 @@ function MergeButtonShell({
  );

  if (!showDropdown) {
-    return compact ? primaryBtn : <span className="self-end">{primaryBtn}</span>;
+    return <span className="self-end">{primaryBtn}</span>;
  }

  return (
-    <span className={compact ? "inline-flex" : "self-end inline-flex"}>
+    <span className="self-end inline-flex">
      {primaryBtn}
      <DropdownMenu>
        <DropdownMenuTrigger asChild>
@@ -148,13 +179,9 @@ function MergeButtonShell({
            aria-label="Choose merge method"
            disabled={disabled}
            onClick={(e) => e.stopPropagation()}
-            className={
-              compact
-                ? "inline-flex items-center rounded-r-full border-l border-green-700/40 bg-green-600 px-1.5 py-0.5 text-white hover:bg-green-700 dark:bg-green-600 dark:hover:bg-green-500 disabled:opacity-60 cursor-pointer"
-                : "inline-flex items-center rounded-r-md border-l border-green-700/40 bg-green-600 px-2 text-white hover:bg-green-700 dark:bg-green-600 dark:hover:bg-green-500 disabled:opacity-60 cursor-pointer"
-            }
+            className="inline-flex items-center rounded-r-md border-l border-green-700/40 bg-green-600 px-2 text-white hover:bg-green-700 dark:bg-green-600 dark:hover:bg-green-500 disabled:opacity-60 cursor-pointer"
          >
-            <IconChevronDown className={compact ? "h-3 w-3" : "h-3.5 w-3.5"} />
+            <IconChevronDown className="h-3.5 w-3.5" />
          </button>
        </DropdownMenuTrigger>
        <DropdownMenuContent align="end" className="w-auto">
@@ -182,6 +209,19 @@ function mergeLabel(method?: MergeMethod): string {
  }
 }

+// Short form used by the compact "or <method>" alternates so the row fits the
+// narrow popover without wrapping.
+function mergeShortLabel(method: MergeMethod): string {
+  switch (method) {
+    case "squash":
+      return "Squash";
+    case "rebase":
+      return "Rebase";
+    case "merge":
+      return "Merge commit";
+  }
+}
+
 // Returns the methods the repo allows, in the order we prefer to surface
 // them (squash → merge → rebase, matching GitHub's own button defaults).
 // When the lookup hasn't resolved (or failed), returns an empty array so
@@ -2,6 +2,7 @@

 import { IconAlertTriangle } from "@tabler/icons-react";
 import { Button } from "@kandev/ui/button";
+import { cn } from "@/lib/utils";
 import type { MergeableState } from "@/lib/types/github";

 // --- Pure descriptor (unit-tested) ---
@@ -67,15 +68,20 @@ function ConflictBanner({
  baseBranch,
  onResolveConflicts,
  resolveDisabled,
+  popover,
 }: {
  baseBranch: string;
  onResolveConflicts?: () => void;
  resolveDisabled?: boolean;
+  popover?: boolean;
 }) {
  return (
    <div
      data-testid="pr-conflict-banner"
-      className="flex items-start gap-2.5 rounded-md border border-red-500/35 bg-red-500/10 px-3 py-2.5"
+      className={cn(
+        "flex items-start gap-2.5 rounded-md border border-red-500/35 bg-red-500/10",
+        popover ? "px-2.5 py-2" : "px-3 py-2.5",
+      )}
    >
      <IconAlertTriangle className="h-4 w-4 shrink-0 text-red-600 dark:text-red-400 mt-0.5" />
      <div className="min-w-0 flex-1">
@@ -102,19 +108,29 @@ function ConflictBanner({
  );
 }

-function MergeabilityChip({ label }: { label: string }) {
+function MergeabilityChip({ label, popover }: { label: string; popover?: boolean }) {
  return (
-    <span className="flex items-center gap-1 text-[10px] text-amber-600 dark:text-amber-400">
-      <IconAlertTriangle className="h-3 w-3" />
+    <span
+      className={cn(
+        "flex items-center text-amber-600 dark:text-amber-400",
+        popover ? "gap-1.5 text-xs" : "gap-1 text-[10px]",
+      )}
+    >
+      <IconAlertTriangle className={popover ? "h-3.5 w-3.5" : "h-3 w-3"} />
      {label}
    </span>
  );
 }

-function NotMergeableText() {
+function NotMergeableText({ popover }: { popover?: boolean }) {
  return (
-    <span className="flex items-center gap-1 text-[10px] text-yellow-600 dark:text-yellow-400">
-      <IconAlertTriangle className="h-3 w-3" />
+    <span
+      className={cn(
+        "flex items-center text-yellow-600 dark:text-yellow-400",
+        popover ? "gap-1.5 text-xs" : "gap-1 text-[10px]",
+      )}
+    >
+      <IconAlertTriangle className={popover ? "h-3.5 w-3.5" : "h-3 w-3"} />
      Not mergeable
    </span>
  );
@@ -128,6 +144,7 @@ export function PRMergeabilityNotice({
  baseBranch,
  onResolveConflicts,
  resolveDisabled,
+  popover,
 }: {
  state: MergeableState | undefined;
  mergeable: boolean;
@@ -136,6 +153,10 @@ export function PRMergeabilityNotice({
  baseBranch: string;
  onResolveConflicts?: () => void;
  resolveDisabled?: boolean;
+  /** Render the CI-hover-popover variant: the banner uses tighter card padding,
+   *  and the chip/text match the popover's `text-xs` rows (so it is slightly
+   *  larger than the detail-panel default, which sits in a denser chip row). */
+  popover?: boolean;
 }) {
  const notice = describeMergeability({ state, mergeable, isDraft, prState });
  if (notice.kind === "none") return null;
@@ -145,11 +166,16 @@ export function PRMergeabilityNotice({
        baseBranch={baseBranch}
        onResolveConflicts={onResolveConflicts}
        resolveDisabled={resolveDisabled}
+        popover={popover}
      />
    );
  return (
-    <div className="flex items-center gap-1.5 flex-wrap">
-      {notice.kind === "chip" ? <MergeabilityChip label={notice.label} /> : <NotMergeableText />}
+    <div className={cn("flex items-center gap-1.5 flex-wrap", popover && "px-1 py-1")}>
+      {notice.kind === "chip" ? (
+        <MergeabilityChip label={notice.label} popover={popover} />
+      ) : (
+        <NotMergeableText popover={popover} />
+      )}
    </div>
  );
 }
@@ -0,0 +1,175 @@
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { cleanup, fireEvent, render } from "@testing-library/react";
+import { StateProvider } from "@/components/state-provider";
+import { ToastProvider } from "@/components/toast-provider";
+import { useCommentsStore, isPRFeedbackComment } from "@/lib/state/slices/comments";
+import { PRMergeabilityRow, blockedReason } from "./pr-mergeability-row";
+import type { AppState } from "@/lib/state/store";
+import type { MergeableState, TaskPR } from "@/lib/types/github";
+
+const SESSION_ID = "sess-1";
+
+function makePR(overrides: Partial<TaskPR> = {}): TaskPR {
+  return {
+    id: "id",
+    task_id: "task-1",
+    owner: "o",
+    repo: "r",
+    pr_number: 7,
+    pr_url: "",
+    pr_title: "Test PR",
+    head_branch: "feat",
+    base_branch: "main",
+    author_login: "alice",
+    state: "open",
+    review_state: "",
+    checks_state: "",
+    mergeable_state: "",
+    review_count: 0,
+    pending_review_count: 0,
+    comment_count: 0,
+    unresolved_review_threads: 0,
+    checks_total: 0,
+    checks_passing: 0,
+    additions: 0,
+    deletions: 0,
+    created_at: "",
+    merged_at: null,
+    closed_at: null,
+    last_synced_at: null,
+    updated_at: "",
+    ...overrides,
+  };
+}
+
+function renderRow(pr: TaskPR, sessionId: string | null = SESSION_ID) {
+  const initialState = {
+    tasks: { activeSessionId: sessionId },
+  } as unknown as Partial<AppState>;
+  return render(
+    <StateProvider initialState={initialState}>
+      <ToastProvider>
+        <div data-testid="row-host">
+          <PRMergeabilityRow pr={pr} />
+        </div>
+      </ToastProvider>
+    </StateProvider>,
+  );
+}
+
+beforeEach(() => {
+  useCommentsStore.setState({
+    byId: {},
+    bySession: {},
+    pendingForChat: [],
+    editingCommentId: null,
+  });
+});
+afterEach(() => cleanup());
+
+describe("PRMergeabilityRow", () => {
+  it("shows the conflict banner with a Resolve conflicts CTA for a dirty PR", () => {
+    const { queryByTestId } = renderRow(makePR({ mergeable_state: "dirty" }));
+    expect(queryByTestId("pr-conflict-banner")).not.toBeNull();
+    expect(queryByTestId("pr-resolve-conflicts-button")).not.toBeNull();
+  });
+
+  it("explains *why* a blocked PR is gated (not just 'Blocked')", () => {
+    const { container, queryByTestId } = renderRow(
+      makePR({ mergeable_state: "blocked", checks_state: "failure" }),
+    );
+    expect(queryByTestId("pr-conflict-banner")).toBeNull();
+    expect(queryByTestId("pr-blocked-note")).not.toBeNull();
+    expect(container.textContent).toContain("Blocked by branch protection");
+  });
+
+  it("stays silent for a blocked PR that is only awaiting a requested review", () => {
+    // The block is just an outstanding reviewer — the review row + calm chip
+    // already convey that, so the row must not show a contradictory note/chip.
+    const { getByTestId } = renderRow(
+      makePR({
+        mergeable_state: "blocked",
+        review_state: "approved",
+        checks_state: "success",
+        pending_review_count: 1,
+      }),
+    );
+    expect(getByTestId("row-host").childElementCount).toBe(0);
+  });
+
+  it("shows a Behind base chip for a behind PR", () => {
+    const { container } = renderRow(makePR({ mergeable_state: "behind" }));
+    expect(container.textContent).toContain("Behind base");
+  });
+
+  it.each(["clean", "unstable", "has_hooks", "unknown", ""] as MergeableState[])(
+    "renders nothing for mergeable_state=%s (stays quiet, no false alarm)",
+    (mergeable_state) => {
+      const { getByTestId } = renderRow(makePR({ mergeable_state }));
+      expect(getByTestId("row-host").childElementCount).toBe(0);
+    },
+  );
+
+  it("renders nothing for a non-open PR even when dirty", () => {
+    const { getByTestId } = renderRow(makePR({ state: "merged", mergeable_state: "dirty" }));
+    expect(getByTestId("row-host").childElementCount).toBe(0);
+  });
+
+  it("queues a conflict-resolution prompt for the active session when the CTA is clicked", () => {
+    const { getByTestId } = renderRow(makePR({ mergeable_state: "dirty", pr_number: 7 }));
+    fireEvent.click(getByTestId("pr-resolve-conflicts-button"));
+
+    const queued = useCommentsStore
+      .getState()
+      .pendingForChat.map((id) => useCommentsStore.getState().byId[id])
+      .filter((c) => !!c && isPRFeedbackComment(c));
+    expect(queued).toHaveLength(1);
+    const comment = queued[0]!;
+    expect(isPRFeedbackComment(comment) && comment.feedbackType).toBe("conflict");
+    expect(isPRFeedbackComment(comment) && comment.prNumber).toBe(7);
+    expect(isPRFeedbackComment(comment) && comment.sessionId).toBe(SESSION_ID);
+    expect(comment.content.toLowerCase()).toContain("conflict");
+  });
+
+  it("hides the Resolve conflicts CTA when there is no active session", () => {
+    const { queryByTestId } = renderRow(makePR({ mergeable_state: "dirty" }), null);
+    // Banner still surfaces the conflict, but the CTA needs a session to target.
+    expect(queryByTestId("pr-conflict-banner")).not.toBeNull();
+    expect(queryByTestId("pr-resolve-conflicts-button")).toBeNull();
+  });
+});
+
+describe("blockedReason", () => {
+  it("names the approval shortfall when required reviews aren't met", () => {
+    expect(blockedReason(makePR({ required_reviews: 2, review_count: 0 }))).toContain(
+      "2 more approvals",
+    );
+    expect(blockedReason(makePR({ required_reviews: 2, review_count: 1 }))).toContain(
+      "1 more approval",
+    );
+  });
+
+  it("points at a failing required check when CI is failure/pending", () => {
+    expect(blockedReason(makePR({ checks_state: "failure" })).toLowerCase()).toContain(
+      "required status check",
+    );
+    expect(blockedReason(makePR({ checks_state: "pending" })).toLowerCase()).toContain(
+      "required status check",
+    );
+  });
+
+  it("does not claim a check failed when no CI is configured (empty checks_state)", () => {
+    // Regression: `checks_state !== "success"` also matched "" and falsely
+    // reported a status-check block on a code-owners/conversation gate.
+    const msg = blockedReason(makePR({ checks_state: "" })).toLowerCase();
+    expect(msg).not.toContain("required status check");
+    expect(msg).toContain("branch protection rule");
+  });
+
+  it("falls back to a generic protection note for other rules", () => {
+    const msg = blockedReason(
+      makePR({ checks_state: "success", required_reviews: 1, review_count: 1 }),
+    );
+    expect(msg.toLowerCase()).toContain("branch protection rule");
+  });
+});
@@ -0,0 +1,134 @@
+"use client";
+
+import { useCallback } from "react";
+import { IconAlertTriangle } from "@tabler/icons-react";
+import { useToast } from "@/components/toast-provider";
+import { useAppStore } from "@/components/state-provider";
+import {
+  useCommentsStore,
+  isPRFeedbackComment,
+  type PRFeedbackComment,
+} from "@/lib/state/slices/comments";
+import type { TaskPR } from "@/lib/types/github";
+import { isPRAwaitingReview } from "./pr-task-icon";
+import { PRMergeabilityNotice, buildConflictResolutionMessage } from "./pr-mergeability-notice";
+
+/**
+ * Wires the "Resolve conflicts" CTA to chat context, mirroring the PR detail
+ * panel (`useAddPRFeedbackAsContext` + `conflictQueued`). Returns a null handler
+ * when there is no active session so the button is suppressed rather than no-op.
+ */
+function useResolveConflicts(pr: TaskPR): {
+  onResolveConflicts: (() => void) | null;
+  conflictQueued: boolean;
+} {
+  const sessionId = useAppStore((s) => s.tasks.activeSessionId);
+  const addComment = useCommentsStore((s) => s.addComment);
+  const { toast } = useToast();
+  const prNumber = pr.pr_number;
+  const headBranch = pr.head_branch;
+  const baseBranch = pr.base_branch;
+
+  // True once a conflict prompt for this PR is already queued — avoids piling
+  // up identical instructions if the user clicks "Resolve conflicts" again.
+  const conflictQueued = useCommentsStore((s) =>
+    s.pendingForChat.some((id) => {
+      const c = s.byId[id];
+      return (
+        !!c &&
+        isPRFeedbackComment(c) &&
+        c.feedbackType === "conflict" &&
+        c.sessionId === sessionId &&
+        c.prNumber === prNumber
+      );
+    }),
+  );
+
+  const handler = useCallback(() => {
+    if (!sessionId || conflictQueued) return;
+    const content = buildConflictResolutionMessage({ prNumber, headBranch, baseBranch });
+    const comment: PRFeedbackComment = {
+      id: `pr-feedback-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`,
+      sessionId,
+      text: content,
+      createdAt: new Date().toISOString(),
+      status: "pending",
+      source: "pr-feedback",
+      prNumber,
+      feedbackType: "conflict",
+      content,
+    };
+    addComment(comment);
+    toast({ description: "Added to chat context" });
+  }, [sessionId, conflictQueued, prNumber, headBranch, baseBranch, addComment, toast]);
+
+  return { onResolveConflicts: sessionId ? handler : null, conflictQueued };
+}
+
+/**
+ * Mergeability cue for the CI hover popover. Surfaces the conflict banner
+ * ("dirty") and the Blocked / Behind-base chips so an unmergeable PR no longer
+ * silently drops its merge button with no explanation. Renders nothing for
+ * clean / unstable / has_hooks and for transient unknown/empty states (we pass
+ * `mergeable: true` so the popover stays quiet instead of flapping "Not
+ * mergeable" while GitHub recomputes).
+ */
+export function PRMergeabilityRow({ pr }: { pr: TaskPR }) {
+  const { onResolveConflicts, conflictQueued } = useResolveConflicts(pr);
+  // "blocked" gets a richer note than the bare chip: it explains *why* the
+  // merge is gated. But when the block is only an outstanding requested review,
+  // stay silent — the review row above already conveys that and the
+  // pill/chip/badge deliberately read it as the calm "awaiting review" state,
+  // so a "Blocked by branch protection" note (or even the bare chip) here would
+  // contradict them.
+  if (pr.state === "open" && pr.mergeable_state === "blocked") {
+    if (isPRAwaitingReview(pr)) return null;
+    return <BlockedNote reason={blockedReason(pr)} />;
+  }
+  return (
+    <PRMergeabilityNotice
+      popover
+      state={pr.mergeable_state}
+      mergeable
+      isDraft={pr.mergeable_state === "draft"}
+      prState={pr.state}
+      baseBranch={pr.base_branch}
+      onResolveConflicts={onResolveConflicts ?? undefined}
+      resolveDisabled={conflictQueued}
+    />
+  );
+}
+
+/**
+ * Best-effort explanation for a `blocked` mergeable state. GitHub doesn't tell
+ * us which branch-protection rule failed, so we infer the common cases from the
+ * fields we do have (required reviews, CI state) and fall back to a generic
+ * note for the rest (code owners, required conversations, etc.).
+ */
+export function blockedReason(pr: TaskPR): string {
+  const required = pr.required_reviews ?? null;
+  if (required != null && pr.review_count < required) {
+    const missing = required - pr.review_count;
+    return `Needs ${missing} more approval${missing === 1 ? "" : "s"} before it can merge.`;
+  }
+  // Only "failure"/"pending" indicate an actual check problem; "" means no CI
+  // is configured (or it hasn't loaded), which isn't a status-check block.
+  if (pr.checks_state === "failure" || pr.checks_state === "pending") {
+    return "A required status check hasn't passed yet.";
+  }
+  return "A branch protection rule (required reviews, checks, or code owners) must be satisfied.";
+}
+
+function BlockedNote({ reason }: { reason: string }) {
+  return (
+    <div data-testid="pr-blocked-note" className="flex items-start gap-1.5 px-1 py-1 text-xs">
+      <IconAlertTriangle className="mt-0.5 h-3.5 w-3.5 shrink-0 text-amber-600 dark:text-amber-400" />
+      <div className="min-w-0">
+        <div className="font-medium text-amber-600 dark:text-amber-400">
+          Blocked by branch protection
+        </div>
+        <p className="text-[11px] leading-snug text-muted-foreground">{reason}</p>
+      </div>
+    </div>
+  );
+}
@@ -224,6 +224,52 @@ describe("PRStatusChip", () => {
  });
 });

+describe("PRStatusChip — mergeability", () => {
+  it("is 'conflict' (not 'passed') for a dirty PR even with green checks + approval", () => {
+    // Regression: the chip read mergeable_state-blind and showed the green
+    // "passed" check on a PR that actually had merge conflicts.
+    renderWithStore(
+      { taskPRs: { byTaskId: { "task-1": [makePR({ mergeable_state: "dirty" })] } } },
+      <PRStatusChip taskId="task-1" />,
+    );
+    const chip = screen.getByTestId(CHIP_TESTID);
+    expect(chip.getAttribute(ATTR_STATUS)).toBe("conflict");
+    expect(chip.getAttribute("data-pr-ready-to-merge")).toBe("false");
+  });
+
+  it("is 'behind' for a behind-base PR that is otherwise green", () => {
+    renderWithStore(
+      { taskPRs: { byTaskId: { "task-1": [makePR({ mergeable_state: "behind" })] } } },
+      <PRStatusChip taskId="task-1" />,
+    );
+    expect(screen.getByTestId(CHIP_TESTID).getAttribute(ATTR_STATUS)).toBe("behind");
+  });
+
+  it("is 'blocked' (amber, not green) for a blocked PR with green checks + approval", () => {
+    renderWithStore(
+      { taskPRs: { byTaskId: { "task-1": [makePR({ mergeable_state: "blocked" })] } } },
+      <PRStatusChip taskId="task-1" />,
+    );
+    expect(screen.getByTestId(CHIP_TESTID).getAttribute(ATTR_STATUS)).toBe("blocked");
+  });
+
+  it("stays 'in_progress' for a blocked PR that is still awaiting a requested review", () => {
+    // Blocked because a reviewer is still pending → that's the awaiting-review
+    // gate, not a generic protection block. Keep the in-progress reading.
+    renderWithStore(
+      {
+        taskPRs: {
+          byTaskId: {
+            "task-1": [makePR({ mergeable_state: "blocked", pending_review_count: 1 })],
+          },
+        },
+      },
+      <PRStatusChip taskId="task-1" />,
+    );
+    expect(screen.getByTestId(CHIP_TESTID).getAttribute(ATTR_STATUS)).toBe("in_progress");
+  });
+});
+
 describe("aggregateChipStatus", () => {
  it("returns 'neutral' for an empty list", () => {
    expect(aggregateChipStatus([])).toBe("neutral");
@@ -240,6 +286,18 @@ describe("aggregateChipStatus", () => {
    const pending = makePR({ id: "pend", review_state: "", checks_state: "pending" });
    expect(aggregateChipStatus([passing, pending])).toBe("in_progress");
  });
+
+  it("lets a conflicting PR dominate a passing sibling", () => {
+    const passing = makePR();
+    const conflict = makePR({ id: "dirty", mergeable_state: "dirty" });
+    expect(aggregateChipStatus([passing, conflict])).toBe("conflict");
+  });
+
+  it("ranks a failing PR above a conflicting one", () => {
+    const conflict = makePR({ id: "dirty", mergeable_state: "dirty" });
+    const failing = makePR({ id: "fail", checks_state: "failure" });
+    expect(aggregateChipStatus([conflict, failing])).toBe("failed");
+  });
 });

 describe("PRStatusChip — multiple PRs", () => {
@@ -7,6 +7,7 @@ import {
  IconChecklist,
  IconLoader2,
  IconPointFilled,
+  IconAlertTriangleFilled,
  IconX,
 } from "@tabler/icons-react";
 import { HoverCard, HoverCardContent, HoverCardTrigger } from "@kandev/ui/hover-card";
@@ -36,10 +37,22 @@ const HOVER_CLOSE_DELAY_MS = 150;

 // Terminal states (merged / closed) never reach here — PRStatusChip returns
 // null for them before rendering — so the chip status union omits them.
-type ChipStatus = "passed" | "failed" | "in_progress" | "neutral";
+type ChipStatus =
+  | "passed"
+  | "failed"
+  | "conflict"
+  | "blocked"
+  | "behind"
+  | "in_progress"
+  | "neutral";

 function chipStatus(pr: TaskPR): ChipStatus {
  if (pr.review_state === "changes_requested" || pr.checks_state === "failure") return "failed";
+  // Merge conflicts / behind-base block the merge even when CI is green — the
+  // chip must never read as a passed check in that case. Mirrors
+  // getPRStatusColor + PRStatusIcon (dirty = red, behind = amber).
+  if (pr.mergeable_state === "dirty") return "conflict";
+  if (pr.mergeable_state === "behind") return "behind";
  // Pending checks / pending review must beat checks_state === "success" so a
  // PR with all checks green but reviewers still outstanding renders as
  // in-progress, not passed. Without this order, the chip flips to green the
@@ -49,14 +62,20 @@ function chipStatus(pr: TaskPR): ChipStatus {
  // Mirror getPRStatusColor priority: ready-to-merge beats awaiting-review so
  // the chip and icon never disagree on a (theoretical) clean+approved+pending PR.
  if (isPRAwaitingReview(pr) && !isPRReadyToMerge(pr)) return "in_progress";
+  // Blocked-by-branch-protection that isn't just an outstanding review (those
+  // are caught above) is a real gate — amber, not the green "passed" check.
+  if (pr.mergeable_state === "blocked") return "blocked";
  if (pr.checks_state === "success") return "passed";
  return "neutral";
 }

 // Higher = more attention-worthy. Drives the aggregate glyph when a task has
-// multiple open PRs — one failing PR colours the whole chip red.
+// multiple open PRs — one failing/conflicting PR colours the whole chip.
 const CHIP_STATUS_RANK: Record<ChipStatus, number> = {
-  failed: 3,
+  failed: 6,
+  conflict: 5,
+  blocked: 4,
+  behind: 3,
  in_progress: 2,
  passed: 1,
  neutral: 0,
@@ -329,6 +348,11 @@ function ChipStatusGlyph({ status }: { status: ChipStatus }) {
      return <IconCircleCheckFilled className="h-3.5 w-3.5 text-green-500" aria-hidden="true" />;
    case "failed":
      return <IconCircleXFilled className="h-3.5 w-3.5 text-red-500" aria-hidden="true" />;
+    case "conflict":
+      return <IconAlertTriangleFilled className="h-3.5 w-3.5 text-red-500" aria-hidden="true" />;
+    case "behind":
+    case "blocked":
+      return <IconAlertTriangleFilled className="h-3.5 w-3.5 text-yellow-500" aria-hidden="true" />;
    case "in_progress":
      // CI runs take minutes, so slow the spin to ~3s/rotation — the default
      // animate-spin (1s) feels frantic for a long-running task.
@@ -45,6 +45,8 @@ function makePR(overrides: Partial<TaskPR> = {}): TaskPR {
 }

 const SKY_400 = "text-sky-400";
+const RED_500 = "text-red-500";
+const YELLOW_500 = "text-yellow-500";

 describe("isPRReadyToMerge", () => {
  it("is true when open + approved + success + clean", () => {
@@ -207,14 +209,16 @@ describe("getPRStatusColor", () => {
    expect(getPRStatusColor(pr)).toBe("text-emerald-400");
  });

-  it("returns plain green for approved+success but mergeable_state blocked", () => {
+  it("returns yellow for approved+success but mergeable_state blocked (branch protection)", () => {
+    // Blocked-by-branch-protection with no outstanding review request is an
+    // attention state — the plain green would imply the PR is good to go.
    const pr = makePR({
      state: "open",
      review_state: "approved",
      checks_state: "success",
      mergeable_state: "blocked",
    });
-    expect(getPRStatusColor(pr)).toBe("text-green-500");
+    expect(getPRStatusColor(pr)).toBe(YELLOW_500);
  });

  it("returns sky-400 for approved PR that still has pending reviewers (1 of N required)", () => {
@@ -296,12 +300,12 @@ describe("getPRStatusColor", () => {
      checks_state: "success",
      mergeable_state: "clean",
    });
-    expect(getPRStatusColor(pr)).toBe("text-red-500");
+    expect(getPRStatusColor(pr)).toBe(RED_500);
  });

  it("returns yellow for pending CI", () => {
    const pr = makePR({ state: "open", checks_state: "pending" });
-    expect(getPRStatusColor(pr)).toBe("text-yellow-500");
+    expect(getPRStatusColor(pr)).toBe(YELLOW_500);
  });

  it("returns purple for merged", () => {
@@ -309,6 +313,37 @@ describe("getPRStatusColor", () => {
  });
 });

+describe("getPRStatusColor — mergeability", () => {
+  it("returns red for a dirty (merge-conflict) PR even when approved + checks pass", () => {
+    // Regression: before the mergeability branch, an approved+success but
+    // conflicted PR fell through to plain green — the icon read "good" while
+    // the PR could not be merged.
+    const pr = makePR({
+      state: "open",
+      review_state: "approved",
+      checks_state: "success",
+      mergeable_state: "dirty",
+    });
+    expect(getPRStatusColor(pr)).toBe(RED_500);
+  });
+
+  it("returns yellow for a behind-base PR that is otherwise approved + green", () => {
+    const pr = makePR({
+      state: "open",
+      review_state: "approved",
+      checks_state: "success",
+      mergeable_state: "behind",
+    });
+    expect(getPRStatusColor(pr)).toBe(YELLOW_500);
+  });
+
+  it("ignores dirty/behind mergeable_state on terminal (merged) PRs", () => {
+    expect(getPRStatusColor(makePR({ state: "merged", mergeable_state: "dirty" }))).toBe(
+      "text-purple-500",
+    );
+  });
+});
+
 describe("isPRAwaitingReview", () => {
  it("is true when CI succeeded and review is pending", () => {
    expect(
@@ -406,7 +441,7 @@ describe("aggregatePRStatusColor", () => {
      review_state: "changes_requested",
      checks_state: "success",
    });
-    expect(aggregatePRStatusColor([green, red])).toBe("text-red-500");
+    expect(aggregatePRStatusColor([green, red])).toBe(RED_500);
  });

  it("returns emerald only when all PRs are ready to merge", () => {
@@ -422,7 +457,7 @@ describe("aggregatePRStatusColor", () => {
  it("yellow CI pending beats merged purple", () => {
    const pending = makePR({ state: "open", checks_state: "pending" });
    const merged = makePR({ state: "merged" });
-    expect(aggregatePRStatusColor([merged, pending])).toBe("text-yellow-500");
+    expect(aggregatePRStatusColor([merged, pending])).toBe(YELLOW_500);
  });

  it("ignores a merged PR when a fresh open PR has no status yet", () => {
@@ -450,7 +485,7 @@ describe("aggregatePRStatusColor", () => {
    const merged = makePR({ state: "merged" });
    expect(aggregatePRStatusColor([merged, merged])).toBe("text-purple-500");
    const closed = makePR({ state: "closed" });
-    expect(aggregatePRStatusColor([closed, closed])).toBe("text-red-500");
+    expect(aggregatePRStatusColor([closed, closed])).toBe(RED_500);
  });
 });

@@ -51,12 +51,26 @@ export function isPRAwaitingReview(pr: TaskPR): boolean {
  return pr.review_state === "pending" || pr.pending_review_count > 0;
 }

+// Colour for the hard merge blockers that must beat ready/awaiting-review:
+// conflicts ("dirty") are a hard stop, "behind" needs a base update first.
+// Returns null for every other state so the caller falls through to its
+// review/check-driven colours. ("blocked" is handled later, after
+// awaiting-review, so an outstanding review still reads as sky.)
+function openMergeBlockerColor(pr: TaskPR): string | null {
+  if (pr.state !== "open") return null;
+  if (pr.mergeable_state === "dirty") return "text-red-500";
+  if (pr.mergeable_state === "behind") return "text-yellow-500";
+  return null;
+}
+
 export function getPRStatusColor(pr: TaskPR): string {
  if (pr.state === "merged") return "text-purple-500";
  if (pr.state === "closed") return "text-red-500";
  if (pr.review_state === "changes_requested" || pr.checks_state === "failure") {
    return "text-red-500";
  }
+  const blockerColor = openMergeBlockerColor(pr);
+  if (blockerColor) return blockerColor;
  if (isPRReadyToMerge(pr)) {
    return "text-emerald-400";
  }
@@ -65,6 +79,12 @@ export function getPRStatusColor(pr: TaskPR): string {
  if (isPRAwaitingReview(pr)) {
    return "text-sky-400";
  }
+  // Blocked by branch protection (a rule other than outstanding reviews, which
+  // the awaiting-review branch above already caught) is an attention state —
+  // not the plain green of an approved+passing PR.
+  if (pr.state === "open" && pr.mergeable_state === "blocked") {
+    return "text-yellow-500";
+  }
  if (pr.review_state === "approved" && pr.checks_state === "success") {
    return "text-green-500";
  }
@@ -1,12 +1,13 @@
 "use client";

-import { memo, useCallback, useEffect, useRef, useState } from "react";
+import { memo, useCallback, useEffect, useRef, useState, type ReactNode } from "react";
 import {
  IconGitPullRequest,
  IconCheck,
  IconX,
  IconClock,
  IconChevronDown,
+  IconAlertTriangle,
 } from "@tabler/icons-react";
 import { Button } from "@kandev/ui/button";
 import { Popover, PopoverAnchor, PopoverContent } from "@kandev/ui/popover";
@@ -37,6 +38,21 @@ import type { TaskPR } from "@/lib/types/github";
 const POPOVER_OPEN_DELAY_MS = 150;
 const POPOVER_CLOSE_DELAY_MS = 150;

+// Badge for the hard merge blockers that must beat ready/awaiting-review:
+// conflicts (red) and behind-base (amber). Mirrors openMergeBlockerColor so
+// the badge agrees with the pill colour. Returns null otherwise. ("blocked"
+// is handled later, after awaiting-review.)
+function mergeBlockerBadge(pr: TaskPR): ReactNode | null {
+  if (pr.state !== "open") return null;
+  if (pr.mergeable_state === "dirty") {
+    return <IconAlertTriangle className="h-3 w-3 text-red-500" />;
+  }
+  if (pr.mergeable_state === "behind") {
+    return <IconAlertTriangle className="h-3 w-3 text-yellow-500" />;
+  }
+  return null;
+}
+
 function PRStatusIcon({ pr }: { pr: TaskPR }) {
  // Terminal states take priority
  if (pr.state === "merged") {
@@ -49,6 +65,8 @@ function PRStatusIcon({ pr }: { pr: TaskPR }) {
  if (pr.checks_state === "failure" || pr.review_state === "changes_requested") {
    return <IconX className="h-3 w-3 text-red-500" />;
  }
+  const blockerBadge = mergeBlockerBadge(pr);
+  if (blockerBadge) return blockerBadge;
  if (isPRReadyToMerge(pr)) {
    return <IconCheck className="h-3 w-3 text-emerald-400" />;
  }
@@ -57,6 +75,11 @@ function PRStatusIcon({ pr }: { pr: TaskPR }) {
  if (isPRAwaitingReview(pr)) {
    return <IconClock className="h-3 w-3 text-sky-400" />;
  }
+  // Blocked by branch protection (not just an outstanding review) → amber,
+  // never the green "all good" check.
+  if (pr.state === "open" && pr.mergeable_state === "blocked") {
+    return <IconAlertTriangle className="h-3 w-3 text-yellow-500" />;
+  }
  if (pr.checks_state === "success" && pr.review_state === "approved") {
    return <IconCheck className="h-3 w-3 text-green-500" />;
  }