From 384ed989a746649c40bde3aaf31d525bd93204fc Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 9 Sep 2026 23:20:09 +0000 Subject: [PATCH] [Chore] Start CodeRabbit review once Ubuntu tests pass --- .github/workflows/label-pr-review-state.yml | 37 ++++- .../pr-review-state-workflow.test.ts | 141 ++++++++++++++++-- 2 files changed, 160 insertions(+), 18 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index d2c24ce3c0..7b7d326765 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -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' : ''} -->` : ''; @@ -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) { @@ -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; } diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index abc77eed2a..5acc4520b1 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -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<{ @@ -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) => ({ @@ -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 })