fix(review): headline a held PR as held, not Closed, when the disposition won't close it

src/queue/processors.ts

@@ -2913,6 +2913,13 @@ async function maybePublishPrPublicSurface(
         unifiedFiles.map((file) => file.path),
         await loadHardGuardrailGlobs(env, repoFullName),
       );
+      // Held-vs-closed parity (#8/#9): the disposition NEVER auto-closes an owner / automation-bot PR, so a gate
+      // "close" verdict on one must headline "held", not "Closed". Compute the same author classification the
+      // planner uses (repo-owner login match + protected automation author) and thread it to the comment.
+      const commentRepoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : "";
+      const commentAuthorLogin = pr.authorLogin ?? "";
+      const neverClosed =
+        (commentAuthorLogin.length > 0 && commentAuthorLogin.toLowerCase() === commentRepoOwner.toLowerCase()) || isProtectedAutomationAuthor(pr.authorLogin);
       const { rows, readinessTotal } = buildPublicPrPanelSignalRows({ repo, pr, profile, detection, queueHealth, collisions, preflight, settings, gate: commentGate, duplicateWinnerEnabled });
       // Visual before/after capture (visual-capture port). Fires ONLY when (1) the global flag + per-repo
       // cutover gate both allow it (screenshotsAllowed) AND (2) the PR touches WEB-VISIBLE files (isVisualPath
@@ -2959,6 +2966,7 @@ async function maybePublishPrPublicSurface(
         ...(aiReview?.reviewerCount !== undefined ? { reviewerCount: aiReview.reviewerCount } : {}),
         mergeReadiness,
         heldForReview,
+        neverClosed,
         extraCollapsibles: buildPublicSafeCollapsibles({
           repo,
           pr,

src/review/unified-comment-bridge.ts

@@ -221,6 +221,9 @@ export type UnifiedCommentBridgeArgs = {
   /** The disposition holds this PR for owner review because its diff touches a hard-guardrail path — so an
    *  otherwise-ready comment renders "held for review" instead of "safe to merge". (#guarded-hold-comment) */
   heldForReview?: boolean | undefined;
+  /** The author is the repo owner or a protected automation bot — never auto-closed, so a gate "close" verdict
+   *  renders as "held" rather than "Closed" (#8/#9). */
+  neverClosed?: boolean | undefined;
 };
 
 /**
@@ -321,6 +324,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
     ...(args.reRunLabel !== undefined ? { reRunLabel: args.reRunLabel } : {}),
     ...(extraCollapsibles !== undefined ? { extraCollapsibles } : {}),
     ...(args.heldForReview ? { heldForReview: true } : {}),
+    ...(args.neverClosed ? { neverClosed: true } : {}),
   });
 
   // Prepend the marker verbatim (matching the legacy body, which leads with the marker then a blank line)

src/review/unified-comment.ts

@@ -206,6 +206,9 @@ export interface UnifiedCommentContext {
   /** The host's disposition holds this PR for owner review (its diff touches a hard-guardrail path), so an
    *  otherwise-ready status renders as "held for review" instead of "safe to merge". (#guarded-hold-comment) */
   heldForReview?: boolean;
+  /** The PR's author is the repo owner or a protected automation bot — the disposition NEVER auto-closes them,
+   *  so a gate "close" verdict renders as "held", not "Closed" (#8/#9). */
+  neverClosed?: boolean;
 }
 
 const STATUS_META: Record<UnifiedCommentStatus, { alert: string; square: string; icon: string }> = {
@@ -269,6 +272,15 @@ export function deriveUnifiedStatus(input: UnifiedReviewInput, ctx: UnifiedComme
   // PR that won't actually merge). Applied LAST so it only ever downgrades an otherwise-ready status — a real
   // CI / merge-state / gate block above still wins. (#guarded-hold-comment)
   if (status === "ready" && ctx.heldForReview) return "held";
+  // Held-vs-closed disposition parity (#8/#9): a gate "close" verdict does NOT always close the PR. An
+  // owner/automation-bot author is NEVER auto-closed, and a guarded-path PR is held for owner review UNLESS a
+  // RED required check forces the close (ciState "failed" mirrors the disposition's redVerifiedRequiredCi). Render
+  // those as "held" so the headline matches the action (#4220 class); a genuine contributor close — red required
+  // CI, or a non-guarded block — still headlines "Closed".
+  if (input.decision === "close") {
+    if (ctx.neverClosed) return "held";
+    if (ctx.heldForReview && input.readiness?.ciState !== "failed") return "held";
+  }
   return status;
 }
 

test/unit/unified-comment-bridge.test.ts

@@ -229,6 +229,18 @@ describe("buildUnifiedCommentBody", () => {
     expect(held).toContain("Held for maintainer review");
     expect(held).not.toContain("> [!TIP]");
   });
+
+  it("neverClosed renders a gate-failure (close) PR as HELD, not 'Closed' (#8/#9)", () => {
+    const args = { gate: gate({ conclusion: "failure" }), panelRows, readinessTotal: 40, changedFiles: 2, mergeReadiness: { ciState: "passed" as const }, footerMarkdown: footer };
+    // A contributor close → the red "Closed" headline.
+    const closed = buildUnifiedCommentBody(args);
+    expect(closed).toContain("Closed");
+    // The SAME verdict on an owner / automation-bot PR (never auto-closed) renders held, not Closed.
+    const held = buildUnifiedCommentBody({ ...args, neverClosed: true });
+    expect(held).toContain("> [!WARNING]");
+    expect(held).toContain("Held for maintainer review");
+    expect(held).not.toContain("Closed");
+  });
 });
 
 // ── Reconciliation invariant (#1016): comment-verdict ↔ gate-conclusion alignment ──────────────────

test/unit/unified-comment.test.ts

@@ -72,12 +72,26 @@ describe("deriveUnifiedStatus", () => {
   it("a guarded-path hold downgrades a would-be-ready PR to held — never 'safe to merge' (#guarded-hold-comment)", () => {
     // A clean+green PR that touches a hard-guardrail path is HELD for owner review, so the comment says held.
     expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } }, { heldForReview: true })).toBe("held");
-    // It only downgrades an otherwise-ready status — a real close/blocked verdict still wins over the hold.
-    expect(deriveUnifiedStatus({ ...base, decision: "close" }, { heldForReview: true })).toBe("blocked");
+    // A guarded close with RED required CI still closes (the red check overrides the guardrail hold), so the
+    // headline stays "blocked"/Closed. The held-vs-closed nuance for non-red guarded closes is covered below.
+    expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "failed" } }, { heldForReview: true })).toBe("blocked");
     // Without the hold flag, the same clean+green PR is ready.
     expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } }, { heldForReview: false })).toBe("ready");
   });
 
+  it("renders a non-closing disposition as held, not Closed (#8/#9)", () => {
+    // #9: an owner / automation-bot author is NEVER auto-closed → a gate "close" verdict renders held, even on red CI.
+    expect(deriveUnifiedStatus({ ...base, decision: "close" }, { neverClosed: true })).toBe("held");
+    expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "failed" } }, { neverClosed: true })).toBe("held");
+    // #8: a guarded-path close is the disposition's HOLD (owner review) unless a red required check forces it.
+    expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "passed" } }, { heldForReview: true })).toBe("held");
+    expect(deriveUnifiedStatus({ ...base, decision: "close" }, { heldForReview: true })).toBe("held"); // CI not yet reported → held
+    expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "failed" } }, { heldForReview: true })).toBe("blocked"); // red required CI → real close
+    // A genuine contributor close (no guard, not owner/bot) still headlines Closed/blocked.
+    expect(deriveUnifiedStatus({ ...base, decision: "close" })).toBe("blocked");
+    expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "failed" } })).toBe("blocked");
+  });
+
   it("honors an explicit host status override", () => {
     expect(deriveUnifiedStatus({ ...base, decision: "close" }, { statusOverride: "ready" })).toBe("ready");
   });