From 229589a001a70c4f19d722e81a0b95d312a0efc9 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 04:28:32 +0000 Subject: [PATCH 1/3] fix(ci): reconcile fork PR review labels --- .github/workflows/label-pr-review-state.yml | 16 ++- .github/workflows/pr-review-event.yml | 14 +++ .../pr-review-state-workflow.test.ts | 98 ++++++++++++------- 3 files changed, 86 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/pr-review-event.yml diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index d2c24ce3c0..96b2840997 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -15,14 +15,13 @@ on: # pull_request_target gives fork PRs a token that can update labels and comments. pull_request_target: types: [opened, reopened, ready_for_review, synchronize, review_requested, labeled, unlabeled] - pull_request_review: - types: [submitted, dismissed] - # Fork review events have a read-only token. CodeRabbit's status-comment update - # provides a trusted base-repository event that can reconcile those PRs promptly. + # Maintainer reviews first trigger the zero-permission PR Review Event workflow. + # workflow_run then reconciles from the trusted default branch with metadata-only access. issue_comment: types: [created, edited] workflow_run: - workflows: ["Code QA Roo Code", "E2E Tests (Mocked)", "Webview Visual Regression", "CodeQL Advanced"] + workflows: + ["PR Review Event", "Code QA Roo Code", "E2E Tests (Mocked)", "Webview Visual Regression", "CodeQL Advanced"] types: [completed] permissions: @@ -112,25 +111,24 @@ jobs: } if (context.eventName === 'workflow_run' && eventPrNumbers.length === 0) { const run = context.payload.workflow_run; - const headOwner = run.head_repository?.owner?.login; - if (headOwner && run.head_branch && run.head_sha) { + if (run.head_branch && run.head_sha) { const candidates = await github.paginate(github.rest.pulls.list, { owner, repo, state: 'open', - head: `${headOwner}:${run.head_branch}`, per_page: 100, }); const baseRepository = `${owner}/${repo}`.toLowerCase(); eventPrNumbers = candidates .filter(pr => pr.head?.sha === run.head_sha && + pr.head?.ref === run.head_branch && pr.base?.repo?.full_name?.toLowerCase() === baseRepository ) .map(pr => pr.number); core.info( `Resolved workflow_run ${run.id ?? '(unknown)'} to ` + - `${eventPrNumbers.length} PR(s) by exact head owner, branch, and SHA` + `${eventPrNumbers.length} PR(s) by exact branch, SHA, and base repository` ); } } diff --git a/.github/workflows/pr-review-event.yml b/.github/workflows/pr-review-event.yml new file mode 100644 index 0000000000..7f31e093e6 --- /dev/null +++ b/.github/workflows/pr-review-event.yml @@ -0,0 +1,14 @@ +name: PR Review Event + +on: + pull_request_review: + types: [submitted, dismissed] + +permissions: {} + +jobs: + notify: + runs-on: ubuntu-latest + steps: + - name: Record review event + run: "true" diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index abc77eed2a..46413d2e6b 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -10,6 +10,9 @@ const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)) const workflow = parse( fs.readFileSync(path.join(repositoryRoot, ".github/workflows/label-pr-review-state.yml"), "utf8"), ) +const reviewEventWorkflow = parse( + fs.readFileSync(path.join(repositoryRoot, ".github/workflows/pr-review-event.yml"), "utf8"), +) const codeRabbitConfig = parse(fs.readFileSync(path.join(repositoryRoot, ".coderabbit.yaml"), "utf8")) const workflowScript = workflow.jobs.reconcile.steps[0].with.script as string @@ -30,9 +33,10 @@ interface HarnessOptions { eventName?: string issueCommentActor?: string workflowRunAssociated?: boolean - workflowRunFallback?: "match" | "sha-mismatch" | "base-mismatch" | "none" + workflowRunFallback?: "match" | "sha-mismatch" | "branch-mismatch" | "base-mismatch" | "none" workflowRunHeadBranch?: string - workflowRunMissing?: "repository" | "branch" | "sha" + workflowRunMissing?: "branch" | "sha" + workflowRunHeadOwner?: string workflowDispatchPrNumber?: number existingGuide?: boolean existingGuideHead?: string @@ -94,7 +98,7 @@ async function runWorkflow(options: HarnessOptions = {}) { draft: options.draft ?? false, html_url: "https://github.com/Zoo-Code-Org/Zoo-Code/pull/1437", user: options.prAuthor ?? { login: "contributor", type: "User" }, - head: { sha: SHA, repo: { full_name: headRepository } }, + head: { ref: "feature/test", sha: SHA, repo: { full_name: headRepository } }, base: { ref: "main", repo: { full_name: "Zoo-Code-Org/Zoo-Code" } }, labels: (options.labels ?? []).map((name) => ({ name })), mergeable: options.mergeable !== undefined ? options.mergeable : options.conflict ? false : true, @@ -241,6 +245,9 @@ async function runWorkflow(options: HarnessOptions = {}) { if (options.workflowRunFallback === "sha-mismatch") { return [{ ...pr, head: { ...pr.head, sha: OLD_SHA } }] } + if (options.workflowRunFallback === "branch-mismatch") { + return [{ ...pr, head: { ...pr.head, ref: "another-branch" } }] + } if (options.workflowRunFallback === "base-mismatch") { return [{ ...pr, base: { ...pr.base, repo: { full_name: "another/repository" } } }] } @@ -356,10 +363,13 @@ async function runWorkflow(options: HarnessOptions = {}) { ? { workflow_run: { pull_requests: options.workflowRunAssociated === false ? [] : [{ number: 1437 }], - head_repository: - options.workflowRunMissing === "repository" - ? null - : { owner: { login: options.fork ? "contributor" : "Zoo-Code-Org" } }, + head_repository: { + owner: { + login: + options.workflowRunHeadOwner ?? + (options.fork ? "contributor" : "Zoo-Code-Org"), + }, + }, head_branch: options.workflowRunMissing === "branch" ? null @@ -445,22 +455,36 @@ describe("PR review-state workflow", () => { expect(result.setFailed).not.toHaveBeenCalled() }) - it("keeps fork review events read-only", async () => { - const result = await runWorkflow({ eventName: "pull_request_review", fork: true }) - - expect(result.addLabels).not.toHaveBeenCalled() - expect(result.removeLabel).not.toHaveBeenCalled() - expect(result.createComment).not.toHaveBeenCalled() - expect(result.updateComment).not.toHaveBeenCalled() - expect(result.createCommitStatus).not.toHaveBeenCalled() - expect(result.setFailed).not.toHaveBeenCalled() + it("bridges review events without permissions or untrusted code execution", () => { + expect(workflow.on.pull_request_review).toBeUndefined() + expect(workflow.on.workflow_run.workflows).toContain("PR Review Event") + expect(reviewEventWorkflow.on.pull_request_review.types).toEqual(["submitted", "dismissed"]) + expect(reviewEventWorkflow.permissions).toEqual({}) + expect(JSON.stringify(reviewEventWorkflow.jobs)).not.toMatch(/actions\/checkout|github\.event|\$\{\{/) }) - it("reconciles same-repository review events", async () => { - const result = await runWorkflow({ eventName: "pull_request_review" }) + it("reconciles maintainer change requests on fork PRs through workflow_run", async () => { + const result = await runWorkflow({ + eventName: "workflow_run", + fork: true, + labels: ["awaiting-maintainer"], + permissions: { maintainer: "write" }, + reviews: [ + { + login: "maintainer", + type: "User", + state: "CHANGES_REQUESTED", + submittedAt: REVIEWED_AT, + }, + ], + }) - expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] })) + expect(result.removeLabel).toHaveBeenCalledWith(expect.objectContaining({ name: "awaiting-maintainer" })) + expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["awaiting-author"] })) expect(result.createCommitStatus).toHaveBeenCalled() + expect(latestGateStatus(result)?.state).toBe("pending") + expect(latestGateStatus(result)?.description).toContain("Address maintainer") + expect(result.setFailed).not.toHaveBeenCalled() }) it("reconciles CodeRabbit status comments with the canonical bot identity", async () => { @@ -1648,9 +1672,8 @@ describe("PR review-state workflow", () => { workflowRunFallback: "match", }) - expect(result.listPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ head: "Zoo-Code-Org:feature/test", state: "open" }), - ) + expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) + expect(result.listPullRequests).not.toHaveBeenCalledWith(expect.objectContaining({ head: expect.anything() })) expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] })) }) @@ -1663,9 +1686,7 @@ describe("PR review-state workflow", () => { }) expect(result.listPullRequests).toHaveBeenCalledTimes(1) - expect(result.listPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ head: "Zoo-Code-Org:feature/test", state: "open" }), - ) + expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) expect(result.getPullRequest).not.toHaveBeenCalled() expect(result.createCommitStatus).not.toHaveBeenCalled() expect(result.addLabels).not.toHaveBeenCalled() @@ -1673,17 +1694,17 @@ describe("PR review-state workflow", () => { expect(result.createLabel).not.toHaveBeenCalled() }) - it("resolves an unassociated fork workflow run by exact head", async () => { + it("resolves an unassociated fork review run when GitHub reports the base repository as head", async () => { const result = await runWorkflow({ eventName: "workflow_run", workflowRunAssociated: false, workflowRunFallback: "match", fork: true, + workflowRunHeadOwner: "Zoo-Code-Org", }) - expect(result.listPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ head: "contributor:feature/test", state: "open" }), - ) + expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) + expect(result.listPullRequests).not.toHaveBeenCalledWith(expect.objectContaining({ head: expect.anything() })) expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] })) }) @@ -1700,6 +1721,19 @@ describe("PR review-state workflow", () => { expect(result.addLabels).not.toHaveBeenCalled() }) + it("ignores an unassociated workflow run when the candidate head branch differs", async () => { + const result = await runWorkflow({ + eventName: "workflow_run", + workflowRunAssociated: false, + workflowRunFallback: "branch-mismatch", + }) + + expect(result.listPullRequests).toHaveBeenCalled() + expect(result.getPullRequest).not.toHaveBeenCalled() + expect(result.createCommitStatus).not.toHaveBeenCalled() + expect(result.addLabels).not.toHaveBeenCalled() + }) + it("ignores an unassociated workflow run when the candidate base repository differs", async () => { const result = await runWorkflow({ eventName: "workflow_run", @@ -1713,7 +1747,7 @@ describe("PR review-state workflow", () => { expect(result.addLabels).not.toHaveBeenCalled() }) - it.each(["repository", "branch", "sha"] as const)( + it.each(["branch", "sha"] as const)( "ignores an unassociated workflow run with missing %s metadata", async (workflowRunMissing) => { const result = await runWorkflow({ @@ -1733,9 +1767,7 @@ describe("PR review-state workflow", () => { const result = await runWorkflow({ eventName: "workflow_run", workflowRunAssociated: false }) expect(result.listPullRequests).toHaveBeenCalledTimes(1) - expect(result.listPullRequests).toHaveBeenCalledWith( - expect.objectContaining({ head: "Zoo-Code-Org:feature/test", state: "open" }), - ) + expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) expect(result.createCommitStatus).not.toHaveBeenCalled() }) From 339248514fc4923831adcf6665c8851e0442808b Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 12:07:42 +0000 Subject: [PATCH 2/3] fix: reject ambiguous workflow run matches --- .github/workflows/label-pr-review-state.yml | 14 ++++++++++---- .../__tests__/pr-review-state-workflow.test.ts | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index 96b2840997..16e4fbf5c7 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -119,13 +119,19 @@ jobs: per_page: 100, }); const baseRepository = `${owner}/${repo}`.toLowerCase(); - eventPrNumbers = candidates - .filter(pr => + const matches = candidates.filter(pr => pr.head?.sha === run.head_sha && pr.head?.ref === run.head_branch && pr.base?.repo?.full_name?.toLowerCase() === baseRepository - ) - .map(pr => pr.number); + ); + if (matches.length === 1) { + eventPrNumbers = [matches[0].number]; + } else if (matches.length > 1) { + core.warning( + `Ignoring ambiguous workflow_run ${run.id ?? '(unknown)'}: ` + + `${matches.length} PRs match the exact branch, SHA, and base repository` + ); + } core.info( `Resolved workflow_run ${run.id ?? '(unknown)'} to ` + `${eventPrNumbers.length} PR(s) by exact branch, SHA, and base repository` diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index 46413d2e6b..4e63e32fdd 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -33,7 +33,7 @@ interface HarnessOptions { eventName?: string issueCommentActor?: string workflowRunAssociated?: boolean - workflowRunFallback?: "match" | "sha-mismatch" | "branch-mismatch" | "base-mismatch" | "none" + workflowRunFallback?: "match" | "ambiguous" | "sha-mismatch" | "branch-mismatch" | "base-mismatch" | "none" workflowRunHeadBranch?: string workflowRunMissing?: "branch" | "sha" workflowRunHeadOwner?: string @@ -242,6 +242,7 @@ async function runWorkflow(options: HarnessOptions = {}) { if (state === "open" && options.prState === "closed") return [] if (eventName === "workflow_run" && options.workflowRunAssociated === false) { if (options.workflowRunFallback === "none" || options.workflowRunFallback === undefined) return [] + if (options.workflowRunFallback === "ambiguous") return [pr, { ...pr, number: pr.number + 1 }] if (options.workflowRunFallback === "sha-mismatch") { return [{ ...pr, head: { ...pr.head, sha: OLD_SHA } }] } @@ -1677,6 +1678,20 @@ describe("PR review-state workflow", () => { expect(result.addLabels).toHaveBeenCalledWith(expect.objectContaining({ labels: ["coderabbit-review-active"] })) }) + it("ignores an ambiguous unassociated workflow run", async () => { + const result = await runWorkflow({ + eventName: "workflow_run", + workflowRunAssociated: false, + workflowRunFallback: "ambiguous", + }) + + expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) + expect(result.getPullRequest).not.toHaveBeenCalled() + expect(result.createCommitStatus).not.toHaveBeenCalled() + expect(result.addLabels).not.toHaveBeenCalled() + expect(result.warning).toHaveBeenCalledWith(expect.stringContaining("Ignoring ambiguous workflow_run")) + }) + it("ignores closed PRs when resolving an unassociated workflow run", async () => { const result = await runWorkflow({ eventName: "workflow_run", From 6c2c0d2db6863a087840974d1a654be55e56002a Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 12:47:50 +0000 Subject: [PATCH 3/3] test(ci): pin fallback PR query --- src/services/__tests__/pr-review-state-workflow.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/services/__tests__/pr-review-state-workflow.test.ts b/src/services/__tests__/pr-review-state-workflow.test.ts index 4e63e32fdd..94ac20f90d 100644 --- a/src/services/__tests__/pr-review-state-workflow.test.ts +++ b/src/services/__tests__/pr-review-state-workflow.test.ts @@ -1685,7 +1685,12 @@ describe("PR review-state workflow", () => { workflowRunFallback: "ambiguous", }) - expect(result.listPullRequests).toHaveBeenCalledWith(expect.objectContaining({ state: "open" })) + expect(result.listPullRequests).toHaveBeenCalledWith({ + owner: "Zoo-Code-Org", + repo: "Zoo-Code", + state: "open", + per_page: 100, + }) expect(result.getPullRequest).not.toHaveBeenCalled() expect(result.createCommitStatus).not.toHaveBeenCalled() expect(result.addLabels).not.toHaveBeenCalled()