Skip to content

[Chore] Start CodeRabbit review once Ubuntu tests pass #1581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
zoomote wants to merge 1 commit into
base: main
Choose a base branch
from
+160 −18
Draft

[Chore] Start CodeRabbit review once Ubuntu tests pass #1581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 31 additions & 6 deletions .github/workflows/label-pr-review-state.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,13 @@ jobs:
}
}

function reviewGuideBody(pr, phase, activationPending = false) {
function reviewGuideBody(pr, phase, activationPending = false, earlyActivation = false) {
const automatedAuthor = pr.user?.type === 'Bot';
const authorNote = automatedAuthor
? 'This PR was opened by an automated account. A human maintainer must verify the change intent, provenance, and validation before merging.'
: 'Thanks for contributing. This comment tracks the review sequence and the next action.';

const labelMarker = phase === 'coderabbit'
const labelMarker = phase === 'coderabbit' || earlyActivation
? `\n${codeRabbitLabelMarkerPrefix}${pr.head.sha}${activationPending ? ':pending' : ''} -->`
: '';

Expand All @@ -412,13 +412,13 @@ jobs:
labelMarker;
}

async function updateReviewGuide(pr, phase, existingGuide = null, activationPending = false) {
async function updateReviewGuide(pr, phase, existingGuide = null, activationPending = false, earlyActivation = false) {
if (isReadOnlyRun && isForkPR(pr)) {
core.info(`PR #${pr.number}: fork PR on a read-only run — skipping guide update`);
return;
}

const body = reviewGuideBody(pr, phase, activationPending);
const body = reviewGuideBody(pr, phase, activationPending, earlyActivation);
const existing = existingGuide ?? await findReviewGuide(pr);

if (!existing) {
Expand Down Expand Up @@ -605,17 +605,42 @@ jobs:
)
);

// CodeRabbit review does not need the full test matrix: the Windows
// jobs run much longer than the Ubuntu jobs, so activation only waits
// for the non-Windows required checks (Ubuntu tests, lint, types, etc.)
// to conclude successfully. A failed Windows run still blocks
// activation until the author pushes a fix.
const windowsTestCheck = context => /\(windows-[^)]*\)/.test(context);
const activationResults = requiredResults.filter(
result => !windowsTestCheck(result.check.context)
);
const activationRuns = activationResults.map(result => result.run).filter(Boolean);
const activationStatuses = activationResults.map(result => result.status).filter(Boolean);
const activationMissing = activationResults.filter(result => !result.run && !result.status);
const codeRabbitActivationPending = requiredChecks === null ||
activationMissing.length > 0 ||
activationRuns.some(run => run.status !== 'completed') ||
activationStatuses.some(status => status.state === 'pending');
const codeRabbitActivationReady = !ciFailed && !codeRabbitActivationPending;

// While CI is running or has failed, remove state labels and move on.
// CI failure is its own signal; the label would add noise, not clarity.
// Once the non-Windows required checks pass, CodeRabbit may still be
// activated early while the Windows test jobs finish.
if (ciPending || ciFailed) {
core.info(`PR #${pr.number}: CI ${ciPending ? 'pending' : 'failed'} — stripping state labels`);
const activateCodeRabbitEarly = ciPending && codeRabbitActivationReady;
const recycleCodeRabbitLabelEarly = activateCodeRabbitEarly &&
codeRabbitLabelHead(existingGuide) !== pr.head.sha;
await updateReviewGate(pr, ciPending ? 'ci-pending' : 'ci-failed', false);
await setCodeRabbitReviewActive(pr, false);
await setCodeRabbitReviewActive(pr, activateCodeRabbitEarly, recycleCodeRabbitLabelEarly);
await reconcileLabels(pr, null);
await updateReviewGuide(
pr,
ciPending ? 'ci-pending' : 'ci-failed',
existingGuide
existingGuide,
false,
activateCodeRabbitEarly
);
continue;
}
Expand Down
141 changes: 129 additions & 12 deletions src/services/__tests__/pr-review-state-workflow.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ interface HarnessOptions {
requiredRunAppId?: number
requiredStatus?: "queued" | "in_progress" | "completed"
requiredConclusion?: "success" | "failure"
requiredRunStates?: Record<
string,
{ status: "queued" | "in_progress" | "completed"; conclusion: "success" | "failure" | null }
>
omitRequiredRuns?: boolean
commitStatuses?: Array<{ context: string; state: "pending" | "success" | "failure" | "error"; id?: number }>
gateStatuses?: Array<{
Expand Down Expand Up @@ -103,18 +107,20 @@ async function runWorkflow(options: HarnessOptions = {}) {
const requiredContexts = options.requiredContexts ?? ["tests"]
const requiredRuns = (options.omitRequiredRuns ? [] : requiredContexts)
.filter((name) => name !== "Zoo Code / reconcile PR review state")
.map((name, index) => ({
id: index + 1,
name,
status: options.requiredStatus ?? "completed",
conclusion:
(options.requiredStatus ?? "completed") === "completed"
? (options.requiredConclusion ?? "success")
: null,
started_at: "2026-08-29T15:00:00Z",
completed_at: (options.requiredStatus ?? "completed") === "completed" ? "2026-08-29T15:01:00Z" : null,
app: { id: options.requiredRunAppId ?? 15368, slug: "github-actions" },
}))
.map((name, index) => {
const state = options.requiredRunStates?.[name]
const status = state?.status ?? options.requiredStatus ?? "completed"
return {
id: index + 1,
name,
status,
conclusion:
status === "completed" ? (state?.conclusion ?? options.requiredConclusion ?? "success") : null,
started_at: "2026-08-29T15:00:00Z",
completed_at: status === "completed" ? "2026-08-29T15:01:00Z" : null,
app: { id: options.requiredRunAppId ?? 15368, slug: "github-actions" },
}
})
const checkRuns = [
...requiredRuns,
...(options.additionalCheckRuns ?? []).map((run) => ({
Expand Down Expand Up @@ -610,6 +616,117 @@ describe("PR review-state workflow", () => {
expect(latestGuide(result)).toContain("a maintainer must restart it")
})

it("starts CodeRabbit once Ubuntu tests pass while Windows tests are still running", async () => {
const result = await runWorkflow({
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "completed", conclusion: "success" },
"platform-unit-test (windows-latest)": { status: "in_progress", conclusion: null },
},
})

expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] }))
expect(result.addLabels).not.toHaveBeenCalledWith(expect.objectContaining({ labels: ["awaiting-coderabbit"] }))
expect(latestGateStatus(result)?.state).toBe("pending")
expect(latestGateStatus(result)?.description).toContain("required CI checks")
expect(latestGuide(result)).toContain(`coderabbit-review-label:${SHA} -->`)
})

it("does not recycle an early CodeRabbit activation bound to the current head", async () => {
const result = await runWorkflow({
labels: ["coderabbit-review-active"],
existingGuideHead: SHA,
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "completed", conclusion: "success" },
"platform-unit-test (windows-latest)": { status: "in_progress", conclusion: null },
},
})

expect(result.removeLabel).not.toHaveBeenCalledWith(
expect.objectContaining({ name: "coderabbit-review-active" }),
)
expect(result.addLabels).not.toHaveBeenCalledWith(
expect.objectContaining({ labels: ["coderabbit-review-active"] }),
)
})

it("recycles an early CodeRabbit activation left over from an older head", async () => {
const result = await runWorkflow({
labels: ["coderabbit-review-active"],
existingGuideHead: OLD_SHA,
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "completed", conclusion: "success" },
"platform-unit-test (windows-latest)": { status: "in_progress", conclusion: null },
},
})

expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "coderabbit-review-active" }))
expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] }))
})

it("does not start CodeRabbit while Ubuntu tests are pending", async () => {
const result = await runWorkflow({
labels: ["coderabbit-review-active"],
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "in_progress", conclusion: null },
"platform-unit-test (windows-latest)": { status: "completed", conclusion: "success" },
},
})

expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "coderabbit-review-active" }))
expect(result.addLabels).not.toHaveBeenCalledWith(
expect.objectContaining({ labels: ["coderabbit-review-active"] }),
)
expect(latestGuide(result)).not.toContain("coderabbit-review-label:")
})

it("does not start CodeRabbit when Ubuntu tests fail", async () => {
const result = await runWorkflow({
labels: ["coderabbit-review-active"],
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "completed", conclusion: "failure" },
"platform-unit-test (windows-latest)": { status: "in_progress", conclusion: null },
},
})

expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "coderabbit-review-active" }))
expect(result.addLabels).not.toHaveBeenCalledWith(
expect.objectContaining({ labels: ["coderabbit-review-active"] }),
)
expect(latestGateStatus(result)?.description).toContain("Fix the failing required CI checks")
})

it("does not start CodeRabbit when Windows tests fail", async () => {
const result = await runWorkflow({
labels: ["coderabbit-review-active"],
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
requiredRunStates: {
"platform-unit-test (ubuntu-latest)": { status: "completed", conclusion: "success" },
"platform-unit-test (windows-latest)": { status: "completed", conclusion: "failure" },
},
})

expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "coderabbit-review-active" }))
expect(result.addLabels).not.toHaveBeenCalledWith(
expect.objectContaining({ labels: ["coderabbit-review-active"] }),
)
expect(latestGateStatus(result)?.description).toContain("Fix the failing required CI checks")
})

it("starts CodeRabbit when Ubuntu and Windows tests have both passed", async () => {
const result = await runWorkflow({
requiredContexts: ["platform-unit-test (ubuntu-latest)", "platform-unit-test (windows-latest)"],
})

expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] }))
expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["awaiting-coderabbit"] }))
expect(latestGuide(result)).toContain(`coderabbit-review-label:${SHA}`)
})

it("invalidates the gate before fallible metadata updates", async () => {
const result = await runWorkflow({ addLabelsStatus: 500 })

Expand Down
Loading