Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
46 changes: 28 additions & 18 deletions .github/workflows/enforce-pr-target.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ on:
- ready_for_review
- synchronize
# A maintainer issue comment ("not touching gui") waives the GUI-screenshot
# gate. `pull_request_target` types do not include issue comments, so a
# separate `issue_comment` trigger re-runs the gate the moment the waiver is
# posted. The gate is idempotent — it re-reads the live PR and updates the
# single consolidated comment — so a comment cannot race or double-mutate.
# gate. CodeRabbit also edits its normal PR status comment when a review
# finishes, which gives this privileged workflow a safe signal to re-check
# review findings even for fork PRs. `pull_request_target` types do not
# include issue comments, so both cases use this separate trigger.
issue_comment:
types:
- created
Expand All @@ -36,14 +36,16 @@ concurrency:

jobs:
enforce-target:
# `issue_comment` fires for comments on ANY issue, PR or not, from ANY
# user. This gate is PR-only and write-capable, so a comment on a plain
# issue — or from a non-maintainer — must not start it. Only a maintainer
# comment on a PR (the GUI-waiver case) may re-run the gate.
# `issue_comment` fires for comments on ANY issue or PR. This gate is
# write-capable, so only two trusted sources may start that path: a
# canonical maintainer (GUI-waiver case) or CodeRabbit's own PR status
# comment, whose create/edit event is used only as a signal to re-read the
# live review threads. All other pull_request_target events run normally.
if: >-
github.event_name != 'issue_comment' ||
(github.event.issue.pull_request != null &&
(github.event.comment.author_association == 'OWNER' ||
(github.event.comment.user.login == 'coderabbitai[bot]' ||
github.event.comment.author_association == 'OWNER' ||
Comment thread
coderabbitai[bot] marked this conversation as resolved.
github.event.comment.author_association == 'COLLABORATOR' ||
github.event.comment.author_association == 'MEMBER'))
runs-on: ubuntu-latest
Expand Down Expand Up @@ -145,6 +147,7 @@ jobs:
const LEGACY_COMMENT_MARKER = "<!-- wrong-branch-enforcer -->";
const REVIEW_READY_LABEL = "review-ready";
const MAINTAINERS_FILE = "MAINTAINERS.md";
const CODE_RABBIT_LOGIN = "coderabbitai[bot]";

const { owner, repo } = context.repo;
// `issue_comment` events carry the PR's issue object, not a
Expand All @@ -155,32 +158,39 @@ jobs:
context.payload.issue?.number;

// Defensive re-check of the job-level guard. `issue_comment` events
// carry a `comment` object with the author's association; a comment
// on a plain issue has no `issue.pull_request`, and a comment from
// anyone but a maintainer must not re-run this write-capable gate.
// carry a `comment` object with the author's association. A normal
// user comment is trusted only when it comes from a canonical
// maintainer; CodeRabbit's own PR status comment is separately
// allowed as a signal to re-read live review threads. The comment
// body itself is never trusted as gate evidence.
if (context.eventName === "issue_comment") {
const isPrComment =
context.payload.issue?.pull_request != null;
const association = context.payload.comment?.author_association;
const commenter = context.payload.comment?.user?.login;
const isCodeRabbit = commenter === CODE_RABBIT_LOGIN;
// The association is a cheap prefilter, but OWNER/COLLABORATOR/
// MEMBER is broader than this repository's canonical maintainer
// list. A collaborator or member who is not a maintainer must not
// start this write-capable gate, so require the commenter's
// login to be in the trusted MAINTAINERS.md list too.
// start this write-capable gate.
const maintainerLogins = new Set(
readMaintainerLogins().map(login => login.toLowerCase())
);
const commenter = context.payload.comment?.user?.login;
const isCanonicalMaintainer =
typeof commenter === "string" &&
maintainerLogins.has(commenter.toLowerCase());
if (
!isPrComment ||
!["OWNER", "COLLABORATOR", "MEMBER"].includes(association) ||
!isCanonicalMaintainer
(!isCodeRabbit &&
(![
"OWNER",
"COLLABORATOR",
"MEMBER"
].includes(association) ||
!isCanonicalMaintainer))
) {
core.info(
"issue_comment not from a canonical maintainer on a PR; skipping the gate."
"issue_comment is neither CodeRabbit nor a canonical maintainer on a PR; skipping the gate."
);
return;
}
Expand Down
47 changes: 47 additions & 0 deletions tests/zz-pr-coderabbit-readiness-revalidation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { describe, expect, test } from "bun:test";

type Workflow = {
on?: {
issue_comment?: { types?: string[] };
};
jobs?: Record<
string,
{
if?: string;
steps?: Array<{
name?: string;
with?: Record<string, string>;
}>;
}
>;
};

describe("CodeRabbit readiness revalidation", () => {
test("CodeRabbit PR status comments can rerun the findings gate", async () => {
const text = await Bun.file(
new URL("../.github/workflows/enforce-pr-target.yml", import.meta.url),
).text();
const workflow = Bun.YAML.parse(text) as Workflow;

expect(workflow.on?.issue_comment?.types).toEqual(["created", "edited"]);

const job = workflow.jobs?.["enforce-target"];
expect(job).toBeDefined();
expect(job?.if).toContain("github.event.issue.pull_request != null");
expect(job?.if).toContain("github.event.comment.user.login == 'coderabbitai[bot]'");

const gateStep = job?.steps?.find(
step => step.name === "Enforce PR target, ancestry, and description",
);
const script = gateStep?.with?.script ?? "";

expect(script).toContain('const CODE_RABBIT_LOGIN = "coderabbitai[bot]"');
expect(script).toContain("const isCodeRabbit = commenter === CODE_RABBIT_LOGIN");
expect(script).toContain("!isCodeRabbit");
expect(script).toContain("isCanonicalMaintainer");
expect(script).toMatch(
/!isPrComment\s*\|\|\s*\(\s*!isCodeRabbit\s*&&\s*\(\s*!\[[\s\S]{0,300}?\.includes\(association\)\s*\|\|\s*!isCanonicalMaintainer\s*\)\s*\)/,
);
expect(script).toContain("unresolvedFindingsClaim");
});
});
Loading