Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se

## Unreleased

- The `hr-pr-followup` review gate (`check-pr-review.ts`) now treats a cubic check on the head
commit like CodeRabbit's. A running cubic check refuses, and `--wait` polls it. A completed one
must conclude SUCCESS, so a NEUTRAL or skipped result, which means cubic did not review the head,
also refuses until someone comments `@cubic review` or its limit clears. A head commit with no
cubic check is not blocked.

- The release procedure (`release-batching` reference) now says to check every package for
commits past its latest release tag before any decision to publish. When there are any, it
recommends a new release first. This covers public and private packages and republishing an
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ changing the configuration.
| `hr-ordering-tests` | Enumerate event orderings through the real code to find race bugs. |
| `hr-plain-prose` | Make text people read plain and specific. |
| `hr-progress-report` | Report current task activity, milestone completion and remaining wall-clock time. |
| `hr-pr-followup` | Address conflicts, feedback and failing checks for a PR; honor landing authority; CodeRabbit review gate. |
| `hr-pr-followup` | Address conflicts, feedback and failing checks for a PR; landing authority; CodeRabbit and cubic gate. |
| `hr-read-reddit` | Read Reddit feed output while supported; RSS support ends November 13, 2026. Partial web-search fallback. |
| `hr-read-x-links` | Read the full content of X posts. |
| `hr-research-shared-links` | Research a shared link or repository in depth, compare it with your setup and rate each idea. |
Expand Down
4 changes: 2 additions & 2 deletions skills/hr-pr-followup/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ Name the exact revision, failed or unfinished gates and unresolved decisions.
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 review gate and one-thread resolver](references/review-helpers.md).
Its review-status checks cover CodeRabbit only; its thread check covers any
[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`).
Expand Down
24 changes: 17 additions & 7 deletions skills/hr-pr-followup/references/review-helpers.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,22 @@ bun scripts/check-pr-review.ts 123 --repo OWNER/REPO

The gate exits 0 when its selected feedback checks pass, 1 for open feedback,
and 2 when coverage cannot be verified. It checks unresolved threads, pending
CodeRabbit review, skipped review statuses and a missing check when the head
contains `.coderabbit.yaml`. It also checks bot review bodies with 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". It does not
check every CI result or authorize landing.
CodeRabbit or cubic review, skipped review statuses and a missing check when
the head contains `.coderabbit.yaml`. It also checks bot review bodies with
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". It does not check every CI result or 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
SUCCESS. A NEUTRAL or skipped conclusion means cubic did not review the head
("Incremental review could not start", "Automatic AI review not started after
branch rewrite", "AI review line limit reached"), so the gate refuses it too.
That pull request stays blocked until someone comments `@cubic review` or the
limit clears, and the refusal says so. A head commit with no cubic check is not blocked, because cubic has no config file
to require. `--wait` does not wait for a cubic check that has not posted yet, so right
after a push, confirm that cubic's check has appeared before landing.

If a review is missing or incomplete, inspect the integration and the repository's
review policy. The helper does not determine automatic-review enablement or
Expand All @@ -42,7 +52,7 @@ 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
runs, or briefly while its expected check has not appeared.
or cubic runs, or briefly while CodeRabbit's expected check has not appeared.

## Resolve one review thread

Expand Down
27 changes: 19 additions & 8 deletions skills/hr-pr-followup/scripts/check-pr-review.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
#!/usr/bin/env bun
// Refuses to land a pull request while review feedback is open: an unresolved review thread, a
// review bot (CodeRabbit) whose check is still running, or a bot review whose body carries findings
// review bot (CodeRabbit or cubic) whose check is still running, or a bot review whose body carries findings
// GitHub cannot show as threads ("Outside diff range comments", "Nitpick comments", "Duplicate comments") with no human
// explicit acknowledgement for that review and the current head. Run only when the repository
// selects this gate. It also refuses when CodeRabbit's status on the
// head commit says anything but "Review completed": CodeRabbit marks a skipped review (for example
// on a non-default base) as success with "Review skipped: …". When the head commit has
// `.coderabbit.yaml` but no CodeRabbit check, it refuses too. Missing or incomplete reviews
// `.coderabbit.yaml` but no CodeRabbit check, it refuses too. A cubic check ("cubic · AI code
// reviewer") on the head commit must also complete with SUCCESS: a running one refuses, and so does
// any other conclusion, NEUTRAL and skipped included, because NEUTRAL means cubic did not review the
// head ("Incremental review could not start", "AI review line limit reached"). That PR stays blocked
// until someone comments `@cubic review` or the limit clears. A head commit with no cubic check is
// not blocked, since cubic has no config file to require, 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.
// 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 review is still running, and for
// the current checkout. --wait polls every 30 s while CodeRabbit's or cubic's review 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.
Expand All @@ -30,13 +36,17 @@
import { spawnSync } from "node:child_process";
import { parseArgs } from "node:util";

const REVIEW_BOTS = /coderabbit/i;
const CODERABBIT = /coderabbit/i;
const CUBIC = /^cubic · AI code reviewer$/i;
const isReviewBot = (name: string) => CODERABBIT.test(name) || CUBIC.test(name);
const REVIEW_BOT_CONFIG = ".coderabbit.yaml";
const BODY_FINDINGS = /Outside diff range comments|Nitpick comments|Duplicate comments/;
const REVIEWED = /^Review completed/;
const FOLLOW_UP = "check the review integration and repository review policy";
const RUNNING = "review still running: ";
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";
const POLL_MS = 30_000;
const NO_CHECK_GRACE_MS = 180_000;

Expand Down Expand Up @@ -364,25 +374,26 @@ export function openFeedback(pr: ReviewState): string[] {
const head = pr.commits.nodes[0]?.commit;
const contexts = head?.statusCheckRollup?.contexts.nodes ?? [];
const hasConfig = head?.tree.entries.some((e) => e.name === REVIEW_BOT_CONFIG) ?? false;
if (hasConfig && !contexts.some((ctx) => REVIEW_BOTS.test(ctx.context ?? ctx.name ?? ""))) {
if (hasConfig && !contexts.some((ctx) => CODERABBIT.test(ctx.context ?? ctx.name ?? ""))) {
open.push(`${NO_CHECK}: ${FOLLOW_UP}`);
}
for (const ctx of contexts) {
const name = ctx.context ?? ctx.name ?? "";
const running =
ctx.state === "PENDING" || (ctx.status !== undefined && ctx.status !== "COMPLETED");
if (!REVIEW_BOTS.test(name)) {
if (!isReviewBot(name)) {
continue;
}
if (running) {
open.push(`${RUNNING}${name}`);
} else if (ctx.name !== undefined && ctx.conclusion !== "SUCCESS") {
const retry = CUBIC.test(name) ? `; ${CUBIC_RETRY}` : "";
open.push(
`${name} review concluded ${ctx.conclusion ?? "(missing conclusion)"}: ${FOLLOW_UP}`,
`${name} review concluded ${ctx.conclusion ?? "(missing conclusion)"}: ${FOLLOW_UP}${retry}`,
);
} else if (
ctx.context !== undefined &&
(ctx.state !== "SUCCESS" || !REVIEWED.test(ctx.description ?? ""))
(ctx.state !== "SUCCESS" || (CODERABBIT.test(name) && !REVIEWED.test(ctx.description ?? "")))
) {
open.push(
`${name} did not review the head commit ("${ctx.description ?? ""}"): ${FOLLOW_UP}`,
Expand Down
56 changes: 56 additions & 0 deletions skills/hr-pr-followup/test/check-pr-review.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,62 @@ test("failed review statuses cannot pass with a completion description", () => {
expect(openFeedback(pr({ contexts: [ctx] }))).toEqual([]);
});

const CUBIC = "cubic · AI code reviewer";
const FOLLOW_UP = "check the review integration and repository review policy";
const CUBIC_RETRY = "comment `@cubic review` or wait for its limit to clear";

// cubic posts a CheckRun and has no config file, so only a check that exists can block.
test("blocks a cubic check that is still running", () => {
for (const status of ["QUEUED", "IN_PROGRESS", "PENDING"]) {
const ctx = { name: CUBIC, status, conclusion: null };
expect(openFeedback(pr({ contexts: [ctx] }))).toEqual([`review still running: ${CUBIC}`]);
}
});

test("blocks a cubic check that concluded without reviewing the head", () => {
for (const conclusion of ["NEUTRAL", "SKIPPED", "FAILURE", "CANCELLED", "TIMED_OUT", null]) {
const ctx = { name: CUBIC, status: "COMPLETED", conclusion };
expect(openFeedback(pr({ contexts: [ctx] }))).toEqual([
`${CUBIC} review concluded ${conclusion ?? "(missing conclusion)"}: ${FOLLOW_UP}; ${CUBIC_RETRY}`,
]);
}
});

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([]);
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([]);
}
});

// cubic's check must not stand in for CodeRabbit's when the repository configures CodeRabbit.
test("a successful cubic check does not satisfy the .coderabbit.yaml presence check", () => {
const done = { name: CUBIC, status: "COMPLETED", conclusion: "SUCCESS" };
expect(openFeedback(pr({ files: [".coderabbit.yaml"], contexts: [done] }))).toEqual([
`no CodeRabbit check on the head commit: ${FOLLOW_UP}`,
]);
});

test("a cubic commit status needs success but no CodeRabbit description", () => {
expect(openFeedback(pr({ contexts: [{ context: CUBIC, state: "SUCCESS" }] }))).toEqual([]);
expect(openFeedback(pr({ contexts: [{ context: CUBIC, state: "FAILURE" }] }))).toHaveLength(1);
});

test("waitForReview polls while cubic runs and stops on its result", async () => {
const running = pr({ contexts: [{ name: CUBIC, status: "IN_PROGRESS", conclusion: null }] });
const neutral = pr({ contexts: [{ name: CUBIC, status: "COMPLETED", conclusion: "NEUTRAL" }] });
let clock = 0;
const opts = { pollMs: 30_000, sleep: async (ms) => (clock += ms), now: () => clock };
const reads = [running, running, neutral];
const open = await waitForReview(() => reads.shift(), { ...opts, timeoutMs: 600_000 });
expect(clock).toBe(60_000);
expect(open).toHaveLength(1);
expect(open[0]).toContain("NEUTRAL");
});

// Hand-written wait loops read a field that stays blank and never stopped; --wait polls this way.
test("waitForReview polls while the review runs and stops when it finishes or time runs out", async () => {
const running = pr({ contexts: [{ context: "CodeRabbit", state: "PENDING" }] });
Expand Down
Loading