fix(review): hold PRs whose review cannot complete, never "safe to merge"

src/queue/processors.ts

@@ -5850,6 +5850,9 @@ async function maybePublishPrPublicSurface(
         mergeReadiness,
         heldForReview,
         neverClosed,
+        // A preflight HOLD (e.g. the review lane is unavailable → the review is incomplete) must never render as
+        // "safe to merge"; the renderer downgrades an otherwise-ready status to a manual-review hold. (#2002)
+        preflightHeld: preflight.status === "hold",
         extraCollapsibles: buildPublicSafeCollapsibles({
           repo,
           pr,

src/review/unified-comment-bridge.ts

@@ -275,6 +275,9 @@ export type UnifiedCommentBridgeArgs = {
   /** 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;
+  /** Preflight is holding this PR (e.g. the review lane is unavailable) — an otherwise-ready comment then renders
+   *  "held", never "safe to merge". (#2002) */
+  preflightHeld?: boolean | undefined;
   /** Public freshness marker for the posted/updated review comment. Defaults to the current publish time. */
   reviewedAt?: string | number | Date | undefined;
 };
@@ -390,6 +393,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
     ...(extraCollapsibles !== undefined ? { extraCollapsibles } : {}),
     ...(args.heldForReview ? { heldForReview: true } : {}),
     ...(args.neverClosed ? { neverClosed: true } : {}),
+    ...(args.preflightHeld ? { preflightHeld: true } : {}),
   });
 
   // Prepend the marker verbatim (matching the legacy body, which leads with the marker then a blank line)

src/review/unified-comment.ts

@@ -209,6 +209,9 @@ export interface UnifiedCommentContext {
   /** 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;
+  /** Preflight is HOLDING this PR (e.g. the review lane is unavailable so the review is incomplete) — an
+   *  otherwise-ready status must then render as "held" (manual review), never "safe to merge". (#2002) */
+  preflightHeld?: boolean;
   /** Public freshness marker for the posted/updated review comment. Rendered as UTC when provided. */
   reviewedAt?: string | number | Date | undefined;
 }
@@ -277,6 +280,12 @@ 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";
+  // A PREFLIGHT HOLD means the review is INCOMPLETE (e.g. the review lane is unavailable) — it otherwise only lands
+  // in the advisory readiness score, so an otherwise-ready status would still read "safe to merge" on an
+  // unfinished review. Downgrade it to a manual-review hold. Applied only to an otherwise-`ready` status, so it can
+  // only ever DOWNGRADE, never approve. (#2002) — NOTE: a gate `merge` verdict WITH advisory blockers stays
+  // authoritative-ready by design (the gate already weighed those); tightening THAT is the gate's confidence/bar.
+  if (status === "ready" && ctx.preflightHeld) return "held";
   // Held-vs-closed disposition parity (#8/#9): owner/automation-bot authors may be exempt from auto-close, so a
   // close verdict on those authors is rendered as held. Guardrail holds are handled above only for otherwise-ready
   // PRs; they must not downgrade a blocker/close verdict to manual review.

src/signals/engine.ts

@@ -4746,7 +4746,7 @@ function validationComponent(pr: PullRequestRecord, preflight: PreflightResult):
   const missingTests = findingCodes.some((code) => /missing.*test|test.*missing|no_test/i.test(code));
   const explicitValidation = hasValidationNote(pr.body ?? "");
   if (preflight.status === "hold") {
-    return { score: 5, evidence: "Preflight is holding this PR; address the blocker before review.", action: "Fix the blocker." };
+    return { score: 5, evidence: "Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.", action: "Await review-lane availability." };
   }
   if (missingTests) {
     // A body validation note is an UNBACKED claim when no test files accompany the change. Cap it just above the

test/unit/signals-coverage.test.ts

@@ -1431,7 +1431,7 @@ describe("signal coverage edge cases", () => {
 
     expect(comment).toContain("> | Linked issue | ✅ No-issue rationale | PR body explains why no issue is linked. | No action. |");
     expect(comment).toContain("> | Change scope | ❌ 8/20 | High review scope from cached public metadata (size label size:L; draft PR; no linked issue context). | Add a concise scope and risk note. |");
-    expect(comment).toContain("> | Validation posture | ❌ 5/25 | Preflight is holding this PR; address the blocker before review. | Fix the blocker. |");
+    expect(comment).toContain("> | Validation posture | ❌ 5/25 | Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review. | Await review-lane availability. |");
     expect(comment).toContain("> | Contributor workload | ✅ 10/10 | Author activity: 29 registered-repo PR(s), 20 merged, 6 issue(s). | No action. |");
     expect(comment).toContain("> | Gate result | ⚠️ Not blocking | Advisory; not blocking this PR. | No action. |");
     expect(comment).toContain("[JSONbored](https://github.com/JSONbored)");

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

@@ -339,6 +339,19 @@ describe("buildUnifiedCommentBody", () => {
     expect(held).not.toContain("> [!TIP]");
   });
 
+  it("preflightHeld renders a passing PR as HELD (incomplete review), never 'safe to merge' (#2002)", () => {
+    const args = { gate: gate({ conclusion: "success" }), panelRows, readinessTotal: 90, changedFiles: 2, mergeReadiness: { ciState: "passed" as const }, footerMarkdown: footer };
+    // Without the preflight hold, a success+green PR is the green approve/merge recommendation.
+    const ready = buildUnifiedCommentBody(args);
+    expect(ready).toContain("Suggested Action - Approve/Merge");
+    // With a preflight hold (e.g. the review lane is unavailable → the review is incomplete), the SAME PR renders
+    // held (WARNING), never safe-to-merge — the incomplete review can't recommend a merge.
+    const held = buildUnifiedCommentBody({ ...args, preflightHeld: true });
+    expect(held).toContain("> [!WARNING]");
+    expect(held).toContain("Suggested Action - Manual Review");
+    expect(held).not.toContain("> [!TIP]");
+  });
+
   it("neverClosed renders a gate-failure (close) PR as HELD when CI is green, not reject/close (#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 reject/close recommendation.

test/unit/unified-comment.test.ts

@@ -48,6 +48,18 @@ describe("deriveUnifiedStatus", () => {
     expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } })).toBe("ready");
   });
 
+  it("never renders 'safe to merge' on an incomplete review — a preflight hold downgrades a gate merge verdict (#2002)", () => {
+    // A preflight HOLD means the review is incomplete (e.g. the review lane is unavailable). A gate `merge` decision
+    // sets `ready` and the hold otherwise only lands in the advisory readiness score — so this downgrade catches it,
+    // and an unfinished review can never read as approve/merge.
+    expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } }, { preflightHeld: true })).toBe("held");
+    // Regression: a clean merge with no hold STILL renders ready — the downgrade only ever downgrades, never approves.
+    expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } })).toBe("ready");
+    // A gate `merge` WITH advisory blockers stays authoritative-ready by design (the gate already weighed them);
+    // tightening that lives in the gate's confidence/approval bars, not this renderer. See the authoritative-merge test.
+    expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" }, blockers: ["minor"] })).toBe("ready");
+  });
+
   it("blocked for a close verdict or consensus blockers", () => {
     expect(deriveUnifiedStatus({ ...base, decision: "close" })).toBe("blocked");
     expect(deriveUnifiedStatus({ ...base, decision: "close", readiness: { ciState: "unverified" } })).toBe("blocked");