From 667ced12e8425f629bacd72857770e9d2d645321 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 14 Oct 2026 00:50:12 +0000 Subject: [PATCH 1/2] fix(ci): reconcile fork PR review labels in a privileged workflow_run follow-up (#2046) --- .github/workflows/label-pr-review-state.yml | 40 ++++++++- .../pr-review-state-workflow.test.ts | 89 +++++++++++++++++++ 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index 2c6674f2c6..5962acf872 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -32,8 +32,15 @@ on: # provides a trusted base-repository event that can reconcile those PRs promptly. issue_comment: types: [created, edited] + # Fork pull_request_review runs get a read-only token and skip their label + # writes. This workflow lists itself so such a run's completion re-triggers + # it with a privileged token that writes the computed labels (issue #2046). + # The resolve job accepts a workflow_run of this workflow only when the + # completed run was itself a pull_request_review run, so the chain stops + # after one hop. The privileged follow-up takes no artifact input and never + # checks out code, so no untrusted PR code or artifacts are touched. workflow_run: - workflows: ["Code QA Roo Code", "E2E Tests (Mocked)", "Webview Visual Regression", "CodeQL Advanced"] + workflows: ["Code QA Roo Code", "E2E Tests (Mocked)", "Webview Visual Regression", "CodeQL Advanced", "Label PR review state"] types: [completed] permissions: @@ -85,9 +92,17 @@ concurrency: jobs: resolve: name: Zoo Code / resolve PRs to reconcile + # The second clause is the loop guard for the self workflow_run + # follow-up (issue #2046): accept a workflow_run of this workflow only + # when the completed run was triggered by pull_request_review. A run + # triggered by workflow_run has event 'workflow_run', so the next level + # is filtered out and the chain stops after one hop. if: >- - github.event_name != 'issue_comment' || - (github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]') + (github.event_name != 'issue_comment' || + (github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]')) && + (github.event_name != 'workflow_run' || + github.event.workflow_run.name != 'Label PR review state' || + github.event.workflow_run.event == 'pull_request_review') runs-on: ubuntu-latest outputs: pr_numbers: ${{ steps.resolve.outputs.pr_numbers }} @@ -125,6 +140,22 @@ jobs: return; } + // Loop guard for the self workflow_run follow-up (issue #2046), + // mirroring the job-level `if`: accept a workflow_run of this + // workflow only when the completed run was triggered by + // pull_request_review. A run triggered by workflow_run has event + // 'workflow_run', so the chain stops after one hop. + if (context.eventName === 'workflow_run' && + context.payload.workflow_run?.name === 'Label PR review state' && + context.payload.workflow_run?.event !== 'pull_request_review') { + core.info( + 'Ignoring workflow_run of this workflow not triggered by a ' + + 'pull_request_review run' + ); + core.setOutput('pr_numbers', '[]'); + return; + } + // Resolve which PRs this event targets. Single-PR events carry the // number in the payload; sweep triggers (hourly schedule, pushes to // main) list every open PR; workflow_run events use the run's @@ -195,6 +226,9 @@ jobs: if: >- (github.event_name != 'issue_comment' || (github.event.issue.pull_request && github.event.comment.user.login == 'coderabbitai[bot]')) && + (github.event_name != 'workflow_run' || + github.event.workflow_run.name != 'Label PR review state' || + github.event.workflow_run.event == 'pull_request_review') && needs.resolve.outputs.pr_numbers != '[]' strategy: fail-fast: false diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index 143ebe3da6..f371c24f82 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -35,6 +35,8 @@ interface HarnessOptions { workflowRunFallback?: "match" | "sha-mismatch" | "base-mismatch" | "none" workflowRunHeadBranch?: string workflowRunMissing?: "repository" | "branch" | "sha" + workflowRunName?: string + workflowRunEvent?: string workflowDispatchPrNumber?: number prNumber?: number runNumber?: number @@ -175,6 +177,11 @@ function buildEventPayload(eventName: string, options: HarnessOptions) { : eventName === "workflow_run" ? { workflow_run: { + // Defaults model a listed CI workflow so existing + // workflow_run tests stay outside the self-run loop + // guard (issue #2046). + name: options.workflowRunName ?? "Code QA Roo Code", + event: options.workflowRunEvent ?? "pull_request", pull_requests: options.workflowRunAssociated === false ? [] @@ -2130,6 +2137,88 @@ describe("PR review-state workflow", () => { expect(resolution.prNumbers()).toEqual([]) }) + describe("fork review self-follow-up (#2046)", () => { + it("writes labels for a fork PR in the trusted run that follows its read-only review run", async () => { + // The workflow lists itself under workflow_run so a fork + // pull_request_review run's completion re-triggers it with a + // privileged token; both jobs gate that self-run on the completed + // run's event. + expect(workflow.on.workflow_run.workflows).toContain("Label PR review state") + expect(String(workflow.jobs.resolve.if)).toContain( + "github.event.workflow_run.event == 'pull_request_review'", + ) + expect(String(workflow.jobs.reconcile.if)).toContain( + "github.event.workflow_run.event == 'pull_request_review'", + ) + + // The fork review run computes the right label but writes nothing. + const readOnly = await runWorkflow({ eventName: "pull_request_review", fork: true }) + + expect(readOnly.addLabels).not.toHaveBeenCalled() + expect(readOnly.removeLabel).not.toHaveBeenCalled() + expect(readOnly.setFailed).not.toHaveBeenCalled() + + // Its completion re-triggers this workflow as a trusted workflow_run. + // Fork runs backfill pull_requests empty, so the resolve job falls + // back to the exact head owner, branch, and SHA lookup. + const resolution = await runResolve({ + eventName: "workflow_run", + fork: true, + workflowRunAssociated: false, + workflowRunFallback: "match", + workflowRunName: "Label PR review state", + workflowRunEvent: "pull_request_review", + }) + + expect(resolution.listPullRequests).toHaveBeenCalledWith( + expect.objectContaining({ head: "contributor:feature/test", state: "open" }), + ) + expect(resolution.prNumbers()).toEqual([1437]) + + // The trusted follow-up shard writes the label the read-only run + // computed but could not write. + const result = await runWorkflow({ eventName: "workflow_run", fork: true }) + + expect(result.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ labels: ["coderabbit-review-active"] }), + ) + expect(result.setFailed).not.toHaveBeenCalled() + }) + + it("ignores a self workflow_run triggered by a non-review run", async () => { + // Every other run of this workflow (hourly sweep, push to main, + // dispatch, pull_request_target) also completes and re-triggers it; + // those follow-ups carry no new review state and are filtered out. + for (const workflowRunEvent of ["schedule", "push", "workflow_dispatch", "pull_request_target"]) { + const resolution = await runResolve({ + eventName: "workflow_run", + workflowRunName: "Label PR review state", + workflowRunEvent, + }) + + expect(resolution.listPullRequests).not.toHaveBeenCalled() + expect(resolution.getPullRequest).not.toHaveBeenCalled() + expect(resolution.prNumbers()).toEqual([]) + } + }) + + it("stops the self workflow_run chain after one hop", async () => { + // The trusted follow-up run completes with event 'workflow_run'. The + // guard requires 'pull_request_review', so its own completion cannot + // queue another run. + const resolution = await runResolve({ + eventName: "workflow_run", + fork: true, + workflowRunName: "Label PR review state", + workflowRunEvent: "workflow_run", + }) + + expect(resolution.listPullRequests).not.toHaveBeenCalled() + expect(resolution.getPullRequest).not.toHaveBeenCalled() + expect(resolution.prNumbers()).toEqual([]) + }) + }) + it("invalidates prior automated and human reviews after a new push", async () => { const result = await runWorkflow({ permissions: { maintainer: "write" }, From a04e0275b027df2acc0a096f3910b569abda5e06 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 17 Oct 2026 01:19:50 +0000 Subject: [PATCH 2/2] test(ci): cover self-run guard for non-review workflow_run events --- .github/workflows/label-pr-review-state.yml | 9 ++++++++- .../__tests__/pr-review-state-workflow.test.ts | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index 5962acf872..c021ad5ca5 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -40,7 +40,14 @@ on: # after one hop. The privileged follow-up takes no artifact input and never # checks out code, so no untrusted PR code or artifacts are touched. workflow_run: - workflows: ["Code QA Roo Code", "E2E Tests (Mocked)", "Webview Visual Regression", "CodeQL Advanced", "Label PR review state"] + workflows: + [ + "Code QA Roo Code", + "E2E Tests (Mocked)", + "Webview Visual Regression", + "CodeQL Advanced", + "Label PR review state", + ] types: [completed] permissions: diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index f371c24f82..b9dba75094 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -2176,7 +2176,9 @@ describe("PR review-state workflow", () => { expect(resolution.prNumbers()).toEqual([1437]) // The trusted follow-up shard writes the label the read-only run - // computed but could not write. + // computed but could not write. The reconcile script reads the PR + // from the matrix number and ignores the workflow_run name and + // event, so the default workflow_run payload is enough here. const result = await runWorkflow({ eventName: "workflow_run", fork: true }) expect(result.addLabels).toHaveBeenCalledWith( @@ -2202,6 +2204,18 @@ describe("PR review-state workflow", () => { } }) + it("does not apply the self-run guard to other workflows that report event workflow_run", async () => { + // Only the 'Label PR review state' name is guarded. A listed CI + // workflow with event 'workflow_run' must still resolve its PR. + const resolution = await runResolve({ + eventName: "workflow_run", + workflowRunName: "Code QA Roo Code", + workflowRunEvent: "workflow_run", + }) + + expect(resolution.prNumbers()).toEqual([1437]) + }) + it("stops the self workflow_run chain after one hop", async () => { // The trusted follow-up run completes with event 'workflow_run'. The // guard requires 'pull_request_review', so its own completion cannot