fix(hr-pr-followup): refuse to merge while a check on the head commit is unfinished or failed

CHANGELOG.md

@@ -15,6 +15,10 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se
 - Repository only (not in the npm package): the end-of-reply eval grader no longer drops a reply's opening
   prose when a scenario allows reads. A reply with a status line, a read-only call and an acting call, and
   no prose after them, is now graded whole. Its required patterns can match the status line.
+- `check-pr-review.ts`, and so `merge-pr.sh`, now refuses while any check on the head commit is
+  unfinished, failed, cancelled or timed out. It used to look only at CodeRabbit and cubic, so a
+  pull request with a red CI check could merge. Neutral and skipped check runs still pass, a head
+  commit with no checks passes, and `--wait` polls while another check runs.
 
 ## 0.15.2 - 2026-10-17
 

skills/hr-pr-followup/SKILL.md

@@ -56,10 +56,11 @@ Clean CI alone is not evidence that every review finding was handled.
 
 When the repository or user selects these GitHub helper policies, use
 [the CodeRabbit and cubic review gate and one-thread resolver](references/review-helpers.md).
-Its review-status checks cover CodeRabbit and cubic only; its thread check covers any
-reviewer and its review-body check any bot. It does not replace other required
-checks or the caller's judgment. The same reference describes the landing helper scripts (`merge-pr.sh`,
-`verify-landed.sh`, `check-landing.py`).
+Its review-status checks cover CodeRabbit and cubic; its thread check covers any
+reviewer and its review-body check any bot. It also refuses while any other check on the head
+commit is unfinished or did not pass. It does not replace the caller's judgment. The same
+reference describes the landing helper scripts (`merge-pr.sh`, `verify-landed.sh`,
+`check-landing.py`).
 
 Immediately before landing, re-read the head, base, checks, review bodies and
 unresolved threads. If the head or base changed, discard readiness and repeat

skills/hr-pr-followup/references/review-helpers.md

@@ -20,8 +20,15 @@ outside-diff, nitpick or duplicate findings. A completed CodeRabbit CheckRun
 must conclude SUCCESS; failed, cancelled, skipped or missing conclusions block
 the gate. A CodeRabbit status also requires SUCCESS and a description starting
 with "Review completed"; that is how the gate tests the general squash rule that an
-existing review check must report completion. It does not check every CI result or
-authorize landing.
+existing review check must report completion.
+
+Every other check on the head commit must have finished and passed. A check run that is
+not COMPLETED, or that concluded anything but SUCCESS, NEUTRAL or SKIPPED, blocks the gate,
+and so does a commit status that is not SUCCESS. A head commit with no checks is not blocked,
+so a repository without CI passes on this point. The gate does not tell required checks from
+optional ones. A check that fails for a known unrelated reason blocks the merge until it passes
+or the owner decides to land another way. `--wait` also polls while such a check runs. The gate
+does not authorize landing.
 
 The check named exactly "cubic · AI code reviewer" on the head commit is held to the
 same rule: while it runs, the gate refuses, and a completed one must conclude
@@ -55,8 +62,8 @@ judges whether the explanation answers the findings.
 
 Connections are paginated. Partial responses, malformed fields, repeated
 cursors and head changes fail closed. A final head read does not make review
-feedback an atomic snapshot. `--wait --timeout 30` polls only while CodeRabbit
-or cubic runs, or briefly while CodeRabbit's expected check has not appeared.
+feedback an atomic snapshot. `--wait --timeout 30` polls while CodeRabbit,
+cubic or any other check runs, or briefly while CodeRabbit's expected check has not appeared.
 
 ## Resolve one review thread
 

skills/hr-pr-followup/scripts/check-pr-review.ts

@@ -14,9 +14,12 @@
 // not blocked, since cubic's `cubic.yaml` is optional and its absence does not show that cubic
 // is expected, and --wait does not wait for one to appear. Missing or incomplete reviews
 // require inspection of the review integration and the repository's selected review policy.
+// Every other check on the head commit must have finished and passed too: a check run that is not
+// COMPLETED, or that concluded anything but SUCCESS, NEUTRAL or SKIPPED, and a commit status that
+// is not SUCCESS, refuse. A head commit with no checks is not blocked.
 //   bun scripts/check-pr-review.ts <pr> [--repo <owner>/<name>] [--wait [--timeout <minutes>]]
 // Exit 1 while feedback is open, 2 when gh fails. Without --repo, gh resolves the repository from
-// the current checkout. --wait polls every 30 s while CodeRabbit's or cubic's review is still running, and for
+// the current checkout. --wait polls every 30 s while CodeRabbit's or cubic's review or any other check is still running, and for
 // up to 3 minutes while the head commit has no CodeRabbit check yet (CodeRabbit posts it a little
 // after a push), then reports as usual; after --timeout minutes (default 30) it reports what is
 // still open.
@@ -44,6 +47,9 @@ const BODY_FINDINGS = /Outside diff range comments|Nitpick comments|Duplicate co
 const REVIEWED = /^Review completed/;
 const FOLLOW_UP = "check the review integration and repository review policy";
 const RUNNING = "review still running: ";
+const CHECK_RUNNING = "check still running: ";
+// Check-run conclusions that do not block a merge; every other conclusion does.
+const PASSING_CONCLUSIONS = ["SUCCESS", "NEUTRAL", "SKIPPED"];
 const NO_CHECK = "no CodeRabbit check on the head commit";
 // A neutral cubic result means it did not review the head; a person has to ask again.
 const CUBIC_RETRY = "comment `@cubic review` or wait for its limit to clear";
@@ -382,6 +388,17 @@ export function openFeedback(pr: ReviewState): string[] {
     const running =
       ctx.state === "PENDING" || (ctx.status !== undefined && ctx.status !== "COMPLETED");
     if (!isReviewBot(name)) {
+      if (ctx.status !== undefined) {
+        if (ctx.status !== "COMPLETED") {
+          open.push(`${CHECK_RUNNING}${name}`);
+        } else if (!PASSING_CONCLUSIONS.includes(ctx.conclusion ?? "")) {
+          open.push(`check ${name} concluded ${ctx.conclusion ?? "(missing conclusion)"}`);
+        }
+      } else if (ctx.state === "PENDING" || ctx.state === "EXPECTED") {
+        open.push(`${CHECK_RUNNING}${name}`);
+      } else if (ctx.state !== "SUCCESS") {
+        open.push(`check ${name} reported ${ctx.state ?? "(missing state)"}`);
+      }
       continue;
     }
     if (running) {
@@ -441,7 +458,8 @@ export function openFeedback(pr: ReviewState): string[] {
   return open;
 }
 
-const reviewRunning = (open: string[]) => open.some((line) => line.startsWith(RUNNING));
+const reviewRunning = (open: string[]) =>
+  open.some((line) => line.startsWith(RUNNING) || line.startsWith(CHECK_RUNNING));
 
 /**
  * Re-reads the pull request while a review is still running, or briefly while its check has not

skills/hr-pr-followup/scripts/merge-pr.sh

@@ -6,7 +6,8 @@
 # worktree or local branch: it lists the worktrees still on the head so their owners clean up.
 #
 # Refuses a PR that is not open, one with open review feedback (check-pr-review.ts: an unresolved
-# thread, a review bot still running, unanswered findings in a bot's review body), or one another
+# thread, a review bot still running, unanswered findings in a bot's review body, any other check on
+# the head commit that is unfinished or did not pass), or one another
 # open PR targets (deleting the head would close that PR; retarget it first with
 # `gh pr edit <n> --base <base>`). The merge passes
 # --match-head-commit, so a push after the head was read fails the merge instead of landing it.

skills/hr-pr-followup/test/check-pr-review.test.js

@@ -34,7 +34,7 @@ test("passes once threads are resolved, the bot is done and body findings were a
     comments: [ack()],
     contexts: [
       { context: "CodeRabbit", state: "SUCCESS", description: "Review completed" },
-      { name: "check", status: "COMPLETED" },
+      { name: "check", status: "COMPLETED", conclusion: "SUCCESS" },
     ],
   });
   expect(openFeedback(state)).toEqual([]);
@@ -70,7 +70,7 @@ test("duplicate body findings still need acknowledgement after the earlier threa
   expect(openFeedback({ ...state, comments: { nodes: [ack()] } })).toEqual([]);
 });
 
-test("blocks unresolved threads and a review bot still running, but not other pending checks", () => {
+test("blocks unresolved threads, a review bot still running and another running check", () => {
   const state = pr({
     threads: [{ isResolved: false, comments: { nodes: [{ url: "https://example.test/t1" }] } }],
     contexts: [
@@ -81,14 +81,15 @@ test("blocks unresolved threads and a review bot still running, but not other pe
   expect(openFeedback(state)).toEqual([
     "unresolved thread: https://example.test/t1",
     "review still running: CodeRabbit",
+    "check still running: build",
   ]);
 });
 
 // A configured review with no check must not pass as "nothing running".
 test("blocks a repository with .coderabbit.yaml whose head commit has no CodeRabbit check", () => {
   const state = pr({
     files: [".coderabbit.yaml", "README.md"],
-    contexts: [{ name: "check", status: "COMPLETED" }],
+    contexts: [{ name: "check", status: "COMPLETED", conclusion: "SUCCESS" }],
   });
   expect(openFeedback(state)).toEqual([
     "no CodeRabbit check on the head commit: check the review integration and repository review policy",
@@ -150,10 +151,14 @@ test("blocks a cubic check that concluded without reviewing the head", () => {
 test("passes a successful cubic check and a repository with no cubic check", () => {
   const done = { name: CUBIC, status: "COMPLETED", conclusion: "SUCCESS" };
   expect(openFeedback(pr({ contexts: [done] }))).toEqual([]);
-  expect(openFeedback(pr({ contexts: [{ name: "check", status: "COMPLETED" }] }))).toEqual([]);
+  expect(
+    openFeedback(pr({ contexts: [{ name: "check", status: "COMPLETED", conclusion: "SUCCESS" }] })),
+  ).toEqual([]);
+  // Names that merely contain "cubic" are ordinary checks: no cubic retry advice, same pass rules.
   for (const other of ["bicubic-resize tests", "cubic-tests", "cubic-lint"]) {
     const similar = { name: other, status: "COMPLETED", conclusion: "FAILURE" };
-    expect(openFeedback(pr({ contexts: [similar] }))).toEqual([]);
+    expect(openFeedback(pr({ contexts: [similar] }))).toEqual([`check ${other} concluded FAILURE`]);
+    expect(openFeedback(pr({ contexts: [{ ...similar, conclusion: "SUCCESS" }] }))).toEqual([]);
   }
 });
 
@@ -209,7 +214,10 @@ test("waitForReview polls while the review runs and stops when it finishes or ti
 // Right after a push CodeRabbit has not posted its check yet; --wait used to give up at once.
 test("waitForReview waits a few minutes for a CodeRabbit check that has not appeared yet", async () => {
   const files = [".coderabbit.yaml"];
-  const missing = pr({ files, contexts: [{ name: "check", status: "COMPLETED" }] });
+  const missing = pr({
+    files,
+    contexts: [{ name: "check", status: "COMPLETED", conclusion: "SUCCESS" }],
+  });
   const running = pr({ files, contexts: [{ context: "CodeRabbit", state: "PENDING" }] });
   const done = pr({
     files,
@@ -229,6 +237,60 @@ test("waitForReview waits a few minutes for a CodeRabbit check that has not appe
   expect(clock).toBe(180_000);
 });
 
+test("blocks any other check that failed, was cancelled or is unfinished, and passes finished ones", () => {
+  const run = (conclusion, status = "COMPLETED") => ({ name: "build", status, conclusion });
+  for (const conclusion of [
+    "FAILURE",
+    "CANCELLED",
+    "TIMED_OUT",
+    "ACTION_REQUIRED",
+    "STARTUP_FAILURE",
+    "STALE",
+  ]) {
+    expect(openFeedback(pr({ contexts: [run(conclusion)] }))).toEqual([
+      `check build concluded ${conclusion}`,
+    ]);
+  }
+  expect(openFeedback(pr({ contexts: [run(null)] }))).toEqual([
+    "check build concluded (missing conclusion)",
+  ]);
+  for (const conclusion of ["SUCCESS", "NEUTRAL", "SKIPPED"]) {
+    expect(openFeedback(pr({ contexts: [run(conclusion)] }))).toEqual([]);
+  }
+  for (const status of ["QUEUED", "IN_PROGRESS", "WAITING", "PENDING", "REQUESTED"]) {
+    expect(openFeedback(pr({ contexts: [run(null, status)] }))).toEqual([
+      "check still running: build",
+    ]);
+  }
+});
+
+test("blocks a commit status that is not SUCCESS", () => {
+  const status = (state) => ({ context: "ci/legacy", state, description: null });
+  expect(openFeedback(pr({ contexts: [status("SUCCESS")] }))).toEqual([]);
+  for (const state of ["PENDING", "EXPECTED"]) {
+    expect(openFeedback(pr({ contexts: [status(state)] }))).toEqual([
+      "check still running: ci/legacy",
+    ]);
+  }
+  for (const state of ["FAILURE", "ERROR"]) {
+    expect(openFeedback(pr({ contexts: [status(state)] }))).toEqual([
+      `check ci/legacy reported ${state}`,
+    ]);
+  }
+});
+
+test("waitForReview polls while another check runs and returns its failure", async () => {
+  const running = pr({ contexts: [{ name: "build", status: "IN_PROGRESS", conclusion: null }] });
+  const failed = pr({ contexts: [{ name: "build", status: "COMPLETED", conclusion: "FAILURE" }] });
+  let clock = 0;
+  const opts = { pollMs: 30_000, sleep: async (ms) => (clock += ms), now: () => clock };
+  const reads = [running, running, failed];
+  expect(await waitForReview(() => reads.shift(), { ...opts, timeoutMs: 600_000 })).toEqual([
+    "check build concluded FAILURE",
+  ]);
+  expect(clock).toBe(60_000);
+});
+
 function ack(overrides = {}) {
   return {
     author: user,