From 0993c53ae67d947c2478063c7730bd2fc27dcf10 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 07:44:58 +0200 Subject: [PATCH] fix(ci): harden comment-driven review workflows --- .github/scripts/enforce-pr-target.test.cjs | 31 ++- .github/workflows/enforce-issue-quality.yml | 2 +- .github/workflows/enforce-pr-target.yml | 151 ++++++------ .github/workflows/pr-hygiene.yml | 4 +- .../000_plan.md | 71 ++++++ .../content/docs/contributing/pr-quality.md | 30 ++- structure/06_docs-and-release.md | 2 +- tests/ci-workflows.test.ts | 214 +++++++++++------- tests/helpers/enforce-pr-target-harness.ts | 46 +++- ...-coderabbit-readiness-revalidation.test.ts | 97 ++++++-- 10 files changed, 429 insertions(+), 219 deletions(-) create mode 100644 devlog/_plan/260808_workflow_comment_spam_hardening/000_plan.md diff --git a/.github/scripts/enforce-pr-target.test.cjs b/.github/scripts/enforce-pr-target.test.cjs index 997333a62..33bfda42c 100644 --- a/.github/scripts/enforce-pr-target.test.cjs +++ b/.github/scripts/enforce-pr-target.test.cjs @@ -49,17 +49,17 @@ describe("enforce-pr-target workflow", () => { assert.match(workflow, /synchronize/); }); - it("re-runs on issue_comment so a maintainer GUI waiver takes effect", () => { - // The GUI-screenshot gate is waived by a maintainer issue comment - // ("not touching gui"). `pull_request_target` types do not include issue - // comments, so without this trigger the waiver sits unread until a PR - // edit or push re-runs the gate. - assert.match(workflow, /^ issue_comment:/m); - assert.match(workflow, /- created/); - assert.match(workflow, /- edited/); - // The script resolves the PR number from the issue payload, which is what - // an issue_comment event delivers instead of a pull_request object. - assert.match(workflow, /context\.payload\.issue\?\.number/); + it("uses label events for GUI waivers and a trusted CodeRabbit status signal", () => { + assert.doesNotMatch(workflow, /^ issue_comment:/m); + assert.match(workflow, /- labeled/); + assert.match(workflow, /- unlabeled/); + assert.match(workflow, /^ status:/m); + assert.match(workflow, /github\.event\.context == 'CodeRabbit'/); + assert.match(workflow, /github\.event\.state == 'success'/); + assert.match(workflow, /github\.event\.label\.name == 'gui-screenshot-waived'/); + assert.match(workflow, /listPullRequestsAssociatedWithCommit/); + assert.match(workflow, /candidate\.head\?\.sha === statusSha/); + assert.match(workflow, /candidates\.length !== 1/); }); it("does not add review events that would break the trusted-base model", () => { @@ -141,13 +141,12 @@ describe("enforce-pr-target workflow", () => { .split("- name: Checkout trusted PR-quality scripts")[1] .split(/\n {6}- name:/)[0]; assert.match(checkoutStep, /actions\/checkout@[0-9a-f]{40}/); - // `pull_request_target` pins the PR base SHA. Privileged `issue_comment` - // runs must source scripts from the repository default branch, matching - // the branch that supplied the workflow itself; unpromoted `dev` scripts - // must never execute under the write-capable token. + // `pull_request_target` pins the PR base SHA. Trusted `status` + // revalidation has no pull_request payload, so it sources scripts from the + // repository default branch that supplied the privileged workflow itself. assert.match( checkoutStep, - /ref:\s*\$\{\{\s*github\.event_name\s*==\s*'issue_comment'\s*&&\s*github\.event\.repository\.default_branch\s*\|\|\s*github\.event\.pull_request\.base\.sha\s*\}\}/, + /ref:\s*\$\{\{\s*github\.event_name\s*==\s*'status'\s*&&\s*github\.event\.repository\.default_branch\s*\|\|\s*github\.event\.pull_request\.base\.sha\s*\}\}/, ); assert.doesNotMatch(checkoutStep, /\|\|\s*'dev'/); // The readiness ping reads MAINTAINERS.md from the same trusted checkout. diff --git a/.github/workflows/enforce-issue-quality.yml b/.github/workflows/enforce-issue-quality.yml index ba828712f..40abd02f9 100644 --- a/.github/workflows/enforce-issue-quality.yml +++ b/.github/workflows/enforce-issue-quality.yml @@ -443,7 +443,7 @@ jobs: translate-comment: name: Translate non-English issue comments - if: github.event_name == 'issue_comment' + if: github.event_name == 'issue_comment' && github.event.issue.pull_request == null && github.event.comment.user.type != 'Bot' runs-on: ubuntu-latest concurrency: # Shares the per-issue queue with the `translate` job: both jobs RMW the diff --git a/.github/workflows/enforce-pr-target.yml b/.github/workflows/enforce-pr-target.yml index cd32bdce1..427ac15d3 100644 --- a/.github/workflows/enforce-pr-target.yml +++ b/.github/workflows/enforce-pr-target.yml @@ -6,17 +6,15 @@ on: - opened - reopened - edited + - labeled + - unlabeled - ready_for_review - synchronize - # A maintainer issue comment ("not touching gui") waives the GUI-screenshot - # 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 - - edited + # CodeRabbit publishes a legacy commit status named `CodeRabbit` on the + # reviewed head SHA. `status` workflows are loaded only from the default + # branch, so a PR cannot suppress or rewrite this signal path. The status is + # only a wake-up signal; the gate re-reads live reviews before any write. + status: # pull-requests:write covers title/comment/label updates. # contents:write is required for convertPullRequestToDraft / @@ -28,26 +26,24 @@ permissions: pull-requests: write concurrency: - # `issue_comment` events carry the issue number, not the PR number. The - # group is shared with the hygiene workflow: both read-modify-write the same - # consolidated gate comment, so serializing them under one key prevents a - # concurrent update from clobbering the other's section. - group: pr-gate-comment-${{ github.event.pull_request.number || github.event.issue.number }} + # PR lifecycle events and CodeRabbit statuses both identify the live head SHA. + # PR hygiene uses that same SHA key, so every writer for one live head is + # serialized even when the wake-up events come from different webhook types. + group: pr-gate-comment-${{ github.event.pull_request.head.sha || github.event.sha || github.run_id }} jobs: enforce-target: - # `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. + # Only CodeRabbit's successful legacy commit status may wake the status + # path. Label events are runner-filtered to the one maintainer-controlled + # waiver label so ordinary type labels and the gate's own `review-ready` + # writes do not allocate another privileged runner. if: >- - github.event_name != 'issue_comment' || - (github.event.issue.pull_request != null && - (github.event.comment.user.login == 'coderabbitai[bot]' || - github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'COLLABORATOR' || - github.event.comment.author_association == 'MEMBER')) + (github.event_name == 'status' && + github.event.context == 'CodeRabbit' && + github.event.state == 'success') || + (github.event_name == 'pull_request_target' && + ((github.event.action != 'labeled' && github.event.action != 'unlabeled') || + github.event.label.name == 'gui-screenshot-waived')) runs-on: ubuntu-latest steps: @@ -60,7 +56,7 @@ jobs: # scripts and MAINTAINERS.md from that same promoted trust boundary. # This prevents unpromoted `dev` script changes from executing with # the workflow's write-capable token. - ref: ${{ github.event_name == 'issue_comment' && github.event.repository.default_branch || github.event.pull_request.base.sha }} + ref: ${{ github.event_name == 'status' && github.event.repository.default_branch || github.event.pull_request.base.sha }} persist-credentials: false sparse-checkout: | .github/scripts @@ -142,54 +138,43 @@ jobs: const TITLE_PREFIX = "[WRONG BRANCH] "; const LEGACY_COMMENT_MARKER = ""; const REVIEW_READY_LABEL = "review-ready"; + const GUI_SCREENSHOT_WAIVER_LABEL = "gui-screenshot-waived"; 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 - // `pull_request` object. The issue number is the PR number either - // way, so resolve it from whichever payload the event delivered. - const pull_number = - context.payload.pull_request?.number ?? - context.payload.issue?.number; + let pull_number = context.payload.pull_request?.number; - // Defensive re-check of the job-level guard. `issue_comment` events - // 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. - const maintainerLogins = new Set( - readMaintainerLogins().map(login => login.toLowerCase()) + // `status` is default-branch controlled, but it carries only the + // reviewed commit SHA. Resolve that SHA back to exactly one open PR + // whose current head still equals the status SHA. Ambiguous or stale + // associations fail closed without mutating any pull request. + if (context.eventName === "status") { + const statusSha = context.payload.sha; + const associatedPrs = await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, + { + owner, + repo, + commit_sha: statusSha, + per_page: 100 + } ); - const isCanonicalMaintainer = - typeof commenter === "string" && - maintainerLogins.has(commenter.toLowerCase()); - if ( - !isPrComment || - (!isCodeRabbit && - (![ - "OWNER", - "COLLABORATOR", - "MEMBER" - ].includes(association) || - !isCanonicalMaintainer)) - ) { + const candidates = associatedPrs.filter( + candidate => + candidate.state === "open" && + candidate.head?.sha === statusSha + ); + if (candidates.length !== 1) { core.info( - "issue_comment is neither CodeRabbit nor a canonical maintainer on a PR; skipping the gate." + `CodeRabbit status ${statusSha} maps to ${candidates.length} open current-head PRs; skipping ambiguous/stale revalidation.` ); return; } + pull_number = candidates[0].number; + } + + if (!Number.isInteger(pull_number)) { + core.info("No pull request could be resolved for this gate event; skipping."); + return; } const { data: pr } = await github.rest.pulls.get({ @@ -490,7 +475,7 @@ jobs: } } - const failures = collectPrQualityFailures({ + let failures = collectPrQualityFailures({ baseRef: pr.base.ref, allowedBases: ALLOWED_BASES, title: pr.title, @@ -511,7 +496,21 @@ jobs: // the GUI waives the screenshot gate. The flag is what tells the // author the screenshot is not required, even though the failure // itself is gone from `failures`. - const screenshotWaived = hasGuiOverride({ comments }); + const screenshotWaivedByLabel = (pr.labels ?? []).some( + label => label.name === GUI_SCREENSHOT_WAIVER_LABEL + ); + if (screenshotWaivedByLabel) { + failures = failures.filter( + failure => failure.code !== "missing_ui_screenshot" + ); + } + const screenshotWaived = + screenshotWaivedByLabel || hasGuiOverride({ comments }); + const screenshotWaiverNotice = screenshotWaived + ? (screenshotWaivedByLabel + ? `UI screenshot waived by the ${inlineCode(GUI_SCREENSHOT_WAIVER_LABEL)} label.` + : "UI screenshot waived by a maintainer comment.") + : null; // The readiness gate applies to contributors (no push permission). // Maintainers keep the failure-only contract: draft while quality @@ -565,7 +564,7 @@ jobs: // the live head only when the event actually delivered it. const eventHeadSha = context.payload.pull_request?.head?.sha ?? - (context.eventName === "issue_comment" + (context.eventName === "status" ? "" : pr.head.sha); const completionHeadSha = @@ -946,9 +945,7 @@ jobs: // the waiver flag, and the prefix notice when the bot owns it. const failureNotices = [ ...revalidationNotice, - ...(screenshotWaived - ? ["UI screenshot waived by a maintainer comment."] - : []), + ...(screenshotWaiverNotice ? [screenshotWaiverNotice] : []), ...(hasWrongBase && state.titlePrefixedByBot ? [`Its title has been prefixed with ${inlineCode(TITLE_PREFIX.trim())}.`] : []) @@ -1038,6 +1035,7 @@ jobs: checklistRequired, notices: [ ...revalidationNotice, + ...(screenshotWaiverNotice ? [screenshotWaiverNotice] : []), "This PR stays in draft until every box above is ticked." ] }); @@ -1057,6 +1055,7 @@ jobs: checklistRequired, notices: [ ...revalidationNotice, + ...(screenshotWaiverNotice ? [screenshotWaiverNotice] : []), "This PR stays in draft until every box above is ticked." ] }); @@ -1089,6 +1088,7 @@ jobs: checklistRequired, notices: [ ...revalidationNotice, + ...(screenshotWaiverNotice ? [screenshotWaiverNotice] : []), "Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked." ] }); @@ -1144,13 +1144,14 @@ jobs: readyState.version = 1; const notices = [ + screenshotWaiverNotice, readyConversionFailed ? "Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft." : readyConverted ? "This pull request has been marked Ready for Review." : "This pull request is already Ready for Review.", readyMoment - ? `CodeRabbit/Codex review was requested via the ${inlineCode(REVIEW_READY_LABEL)} label. If no review appears, comment ${inlineCode("@coderabbitai review")} to request one.` + ? `The ${inlineCode(REVIEW_READY_LABEL)} label marks this PR as ready; review automation runs independently. If no CodeRabbit review appears, comment ${inlineCode("@coderabbitai review")} to request one.` : "", notified && maintainers.length > 0 ? `Maintainers notified: ${maintainers @@ -1214,7 +1215,7 @@ jobs: actions: [], readiness, checklistRequired, - notices: [] + notices: screenshotWaiverNotice ? [screenshotWaiverNotice] : [] }); return; } @@ -1227,7 +1228,7 @@ jobs: actions: [], readiness, checklistRequired, - notices: [] + notices: screenshotWaiverNotice ? [screenshotWaiverNotice] : [] } ); return; diff --git a/.github/workflows/pr-hygiene.yml b/.github/workflows/pr-hygiene.yml index 7d42712cb..4820c6db3 100644 --- a/.github/workflows/pr-hygiene.yml +++ b/.github/workflows/pr-hygiene.yml @@ -11,11 +11,11 @@ permissions: {} concurrency: # Shared with the enforce-target gate: both workflows read-modify-write the - # same consolidated gate comment, so one per-PR group serializes them. + # same consolidated gate comment, so one live-head SHA group serializes them. # `cancel-in-progress` stays false (the enforce-target gate also omits it): # a newer run must queue behind the in-flight one, never cancel it mid # comment mutation, or the cancelled run's read-modify-write is lost. - group: pr-gate-comment-${{ github.event.pull_request.number }} + group: pr-gate-comment-${{ github.event.pull_request.head.sha }} cancel-in-progress: false jobs: diff --git a/devlog/_plan/260808_workflow_comment_spam_hardening/000_plan.md b/devlog/_plan/260808_workflow_comment_spam_hardening/000_plan.md new file mode 100644 index 000000000..a12e5883a --- /dev/null +++ b/devlog/_plan/260808_workflow_comment_spam_hardening/000_plan.md @@ -0,0 +1,71 @@ +# Workflow comment-spam hardening implementation plan + +> **For agentic workers:** execute this plan test-first. Do not broaden workflow permissions or execute pull-request head code with a write-capable token. + +**Goal:** Reduce GitHub Actions noise and runner consumption caused by `issue_comment` while preserving issue-comment translation and the PR readiness gate's ability to invalidate a ready PR when CodeRabbit reports new findings. + +**Architecture:** Keep `issue_comment` only where GitHub offers no narrower native trigger: real-time issue-comment translation. Revalidate CodeRabbit readiness from its `CodeRabbit` commit status using the default-branch-only `status` event, then resolve the status SHA to exactly one open PR before the privileged gate writes anything. Make `gui-screenshot-waived` the immediate maintainer-controlled waiver trigger while preserving legacy maintainer-comment recognition on later PR events for compatibility. + +**Tech stack:** GitHub Actions YAML, `actions/github-script`, Bun tests, existing PR-gate scripts. + +## Global constraints + +- PR targets `dev`. +- Workflow changes become live only after promotion to default branch `main`. +- Never checkout or execute PR-head code in a workflow with write permissions. +- Preserve real-time non-English issue-comment translation. +- Preserve CodeRabbit/Codex review-thread verification as the source of truth; review/comment bodies are trigger signals only, never trusted gate evidence. +- Do not claim that a job-level `if` removes an `issue_comment` workflow-run entry: it only prevents runner allocation for filtered comments. + +## Task 1: Stop PR and bot comments from allocating issue-quality runners + +**Files:** +- Modify: `.github/workflows/enforce-issue-quality.yml` +- Modify: `tests/ci-workflows.test.ts` + +- [ ] Add regression assertions requiring the `translate-comment` job to run only for `issue_comment` events on real issues and non-bot authors. +- [ ] Run the focused workflow test and confirm it fails against the current workflow. +- [ ] Add the minimal job-level guard: exclude `github.event.issue.pull_request != null` and bot-authored comments before checkout/setup/AI steps. +- [ ] Re-run the focused workflow test and confirm it passes. + +## Task 2: Replace CodeRabbit status-comment gate triggers with a trusted commit-status signal + +**Files:** +- Modify: `.github/workflows/enforce-pr-target.yml` +- Replace: `tests/zz-pr-coderabbit-readiness-revalidation.test.ts` + +- [ ] Require no `issue_comment`, `pull_request_review`, or PR-controlled signal workflow for CodeRabbit revalidation. +- [ ] Consume CodeRabbit's successful `CodeRabbit` commit status through the default-branch-only `status` event. +- [ ] Resolve the status SHA with `listPullRequestsAssociatedWithCommit` and continue only when exactly one open PR has that SHA as its current head. +- [ ] Treat status-triggered runs as signal-only head evidence and re-read live review threads/bodies before any write. +- [ ] Keep the write-capable checkout pinned to the trusted default branch for status events. + +## Task 3: Move GUI screenshot waiver from maintainer comments to a label + +**Files:** +- Modify: `.github/workflows/enforce-pr-target.yml` +- Modify: `tests/ci-workflows.test.ts` +- Modify: `docs-site/src/content/docs/contributing/pr-quality.md` + +- [ ] Add regression assertions for `labeled` / `unlabeled` PR-target events and `gui-screenshot-waived` semantics. +- [ ] Confirm the new assertions fail against current behavior. +- [ ] Use `gui-screenshot-waived` as the only immediate GUI-waiver trigger, while preserving legacy maintainer-comment recognition on later PR events for compatibility. +- [ ] Document that the label is maintainer-controlled and that adding/removing it immediately re-evaluates the gate. +- [ ] Re-run focused workflow tests. + +## Task 4: Verification and PR + +- [ ] Run `bun test tests/zz-pr-coderabbit-readiness-revalidation.test.ts tests/ci-workflows.test.ts`. +- [ ] Run `node --test .github/scripts/*.test.cjs` because the gate still consumes those helpers. +- [ ] Run `bun run typecheck`. +- [ ] Run `git diff --check`. +- [ ] Verify the final diff contains no temporary implementation workflow or helper. +- [ ] Open a draft PR against `dev` with deployment note: event-driven workflow changes take effect only after promotion to `main`. + +## Expected effect + +- CodeRabbit PR status-comment edits no longer invoke `enforce-pr-target`. +- Ordinary maintainer PR comments no longer invoke `enforce-pr-target` merely to carry a GUI waiver. +- PR and bot comments still create an `Enforce issue quality` workflow-run record because GitHub cannot filter `issue_comment` by PR-vs-issue at trigger time, but the translation job is skipped before runner allocation. +- Real issue comments from humans continue to translate in real time. +- New CodeRabbit reviews can still invalidate a previously completed findings claim through CodeRabbit's commit status and a default-branch, write-capable gate without executing untrusted PR code or trusting an ambiguous SHA-to-PR association. diff --git a/docs-site/src/content/docs/contributing/pr-quality.md b/docs-site/src/content/docs/contributing/pr-quality.md index a7948d96b..e29619e5a 100644 --- a/docs-site/src/content/docs/contributing/pr-quality.md +++ b/docs-site/src/content/docs/contributing/pr-quality.md @@ -40,12 +40,12 @@ tells you exactly what to change: plan** (or equivalent substance). When the title or description mentions `gui`, the description must include a screenshot of the UI change; the check keeps the PR a draft and comments until the screenshot is present. A - maintainer (OWNER / COLLABORATOR / MEMBER — repository owners, - collaborators, and members) can waive the screenshot - requirement with an issue comment saying the change does not touch the GUI - (for example "no gui changes"); a contributor PR author cannot self-waive - (a maintainer who authors the PR can waive, but they already hold push - permission and are not gated by the contributor checklist). + maintainer can waive a false-positive GUI cue by adding the + `gui-screenshot-waived` label; adding or removing that label immediately + re-evaluates the gate. Legacy maintainer comments such as "no gui changes" + are still recognised on the next PR event for compatibility, but comments + themselves no longer trigger the privileged PR gate. A contributor cannot + self-waive the screenshot requirement. Contributor PRs (authors without repository push permission) open in draft and stay there until a four-box review-readiness checklist in the description is complete: local CI green, the branch on the latest `dev` @@ -71,6 +71,13 @@ tells you exactly what to change: unticks the matching box and keeps the PR a draft. When the checklist is complete and every gate is green, the gate adds a `review-ready` label as a visible status marker at the ready moment. + CodeRabbit status-comment edits do not trigger the PR gate. CodeRabbit's + successful `CodeRabbit` commit status wakes the trusted default-branch gate + through the `status` event. The gate maps that status SHA to exactly one open + PR whose current head still matches, then re-reads live review threads and + review bodies before changing checklist, label, comment, or draft state. An + ambiguous or stale SHA association is ignored, and no PR-head code is + executed with the gate's write-capable token. - **Hygiene.** Behavior changes need a test; new lint or type suppressions, focused or skipped tests, empty catch blocks, edited generated output, and a @@ -98,11 +105,12 @@ right; say why when it is wrong. It does not block a merge. ### When a workflow change takes effect -`enforce-target` and `label` run on `pull_request_target`, which GitHub always -loads from the repository **default branch**. A change to either takes effect -only after it is promoted to `main` — merging it to `dev` does not change live -behavior. The cross-platform CI workflow runs on `pull_request` and takes effect -as soon as it is on the branch being targeted. +`enforce-target` and `label` use trusted default-branch automation. The PR gate +runs on `pull_request_target` and on CodeRabbit `status` events, both loaded +from the repository default branch; the write-capable behavior therefore +changes only after the gate revision is promoted to `main`. The cross-platform +CI workflow runs on `pull_request` and takes effect as soon as it is on the +branch being targeted. ## Sponsored surfaces diff --git a/structure/06_docs-and-release.md b/structure/06_docs-and-release.md index 8274487b5..63b053032 100644 --- a/structure/06_docs-and-release.md +++ b/structure/06_docs-and-release.md @@ -44,7 +44,7 @@ bun run build | `.github/workflows/release.yml` | Manual dispatch only | npm publish/dry-run workflow. It requires the exact `GITHUB_SHA` to have a successful Cross-platform CI run before publish or dry-run. | | `.github/workflows/deploy-docs.yml` | `push` to `main` touching `docs-site/**` or the workflow, or manual dispatch | Build and publish the Astro/Starlight docs site to GitHub Pages. | | `.github/workflows/service-lifecycle.yml` | `pull_request` to `main`/`dev` and `push`, both filtered on the service path set (`src/service.ts`, `src/cli.ts`, `src/cli/index.ts`, `src/lib/bun-runtime.ts`, `package.json`, `bun.lock`, the workflow), or manual dispatch | Service-lifecycle smoke on three platforms: Linux systemd, macOS launchd, and Windows Scheduled Tasks. Each installs, verifies, stops via `ocx stop`, and uninstalls. The path list is kept in sync with the `release.yml` service-gate regex. | -| `.github/workflows/enforce-pr-target.yml` | `pull_request_target` (opened, reopened, edited, ready_for_review, synchronize) | The `enforce-target` gate: rejects pull requests whose head ancestry sits on the `main` tip while far behind `dev`, rejects empty or malformed descriptions, requires a GUI screenshot when the title/body mentions `gui` (waivable by a maintainer comment), keeps contributor PRs in draft until a four-box readiness checklist is complete, verifies the CI / latest-dev / Codex+CodeRabbit-findings claims (review threads plus current-head CodeRabbit review-body findings outside the diff range), and adds a `review-ready` status label at the ready moment. Stacked child PRs targeting another open PR's head skip the wrong-base gate. | +| `.github/workflows/enforce-pr-target.yml` | `pull_request_target` (opened, reopened, edited, labeled, unlabeled, ready_for_review, synchronize) plus default-branch `status` events filtered to successful `CodeRabbit` statuses | The `enforce-target` gate: rejects pull requests whose head ancestry sits on the `main` tip while far behind `dev`, rejects empty or malformed descriptions, requires a GUI screenshot when the title/body mentions `gui` (immediately waivable with the maintainer-controlled `gui-screenshot-waived` label; legacy maintainer comments remain compatibility evidence on later PR events), keeps contributor PRs in draft until a four-box readiness checklist is complete, verifies the CI / latest-dev / Codex+CodeRabbit-findings claims (review threads plus current-head CodeRabbit review-body findings outside the diff range), and adds a `review-ready` status label at the ready moment. CodeRabbit status SHAs must resolve to exactly one open current-head PR before writes. Stacked child PRs targeting another open PR's head skip the wrong-base gate. | | `.github/workflows/enforce-issue-quality.yml` | `issues` (opened, edited, reopened), `issue_comment` (created, edited), or manual dispatch with an issue number | Issue-template compliance gate. | | `.github/workflows/issue-quality-tests.yml` | `pull_request` and `push` filtered on the issue/PR automation scripts, templates, and their workflows | Tests the issue and PR automation scripts themselves, so the gates cannot rot silently. | | `.github/workflows/issue-triage.yml` | `issues` (opened) | Duplicate detection and triage labeling for new issues. | diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index 67be862ea..f9d7856c8 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -679,6 +679,10 @@ describe("GitHub Actions hardening", () => { on?: { pull_request_target?: { types?: string[] }; issue_comment?: { types?: string[] }; + workflow_run?: { workflows?: string[]; types?: string[] }; + pull_request_review?: { types?: string[] }; + pull_request_review_comment?: { types?: string[] }; + status?: unknown; }; permissions?: Record | string; concurrency?: Record & { group?: string }; @@ -839,14 +843,12 @@ describe("GitHub Actions hardening", () => { // workflow YAML under a write token against base-pinned scripts — a // mismatch that crashes the gate and breaks the trusted-base model. // - // `issue_comment` is the one extra trigger: a maintainer's GUI-waiver - // comment ("not touching gui") must re-run the gate, and issue comments - // are not a `pull_request_target` activity type. It never touches PR head - // code — the checkout stays on the trusted base/default branch — so it - // does not open the escalation path review events would. + // `status` is the only extra trigger. CodeRabbit publishes a legacy + // commit status; this privileged workflow is loaded from the default branch + // and re-reads live review evidence before any mutation. expect(Object.keys(workflow.on ?? {}).sort()).toEqual([ - "issue_comment", "pull_request_target", + "status", ]); // And the trigger is exactly a `types:` list — nothing else. @@ -859,7 +861,7 @@ describe("GitHub Actions hardening", () => { // additive, both look like ordinary scoping in a diff, and neither failed a // single assertion. expect(Object.keys(workflow.on?.pull_request_target ?? {})).toEqual(["types"]); - expect(Object.keys(workflow.on?.issue_comment ?? {})).toEqual(["types"]); + expect(Object.prototype.hasOwnProperty.call(workflow.on ?? {}, "status")).toBe(true); // Exactly the scopes this gate needs. `pull-requests: write` covers title // and comment updates. `contents: write` is required for the draft GraphQL @@ -871,14 +873,13 @@ describe("GitHub Actions hardening", () => { "pull-requests": "write", }); - // One run per PR, so two rapid events cannot race on the title/draft state, - // and no `cancel-in-progress` — cancelling the in-flight run mid-mutation is - // how the bot ends up having prefixed the title but not recorded that it did. - // `issue_comment` events carry the PR's number under `issue`, not - // `pull_request`, so the group resolves from whichever payload exists. + // Every writer for one live PR head must resolve to the same SHA lock. + // `pull_request_target` exposes it under pull_request.head.sha; `status` + // exposes the same value as event.sha. The run-id fallback is fail-safe for + // malformed payloads and prevents unrelated runs sharing an empty key. expect(workflow.concurrency).toEqual({ group: - "pr-gate-comment-${{ github.event.pull_request.number || github.event.issue.number }}", + "pr-gate-comment-${{ github.event.pull_request.head.sha || github.event.sha || github.run_id }}", }); // The hygiene workflow reads and rewrites the same consolidated gate @@ -889,8 +890,10 @@ describe("GitHub Actions hardening", () => { await readText(".github/workflows/pr-hygiene.yml"), ) as { concurrency?: { group?: string; "cancel-in-progress"?: boolean } }; expect(hygieneWorkflow.concurrency?.group).toBe( - "pr-gate-comment-${{ github.event.pull_request.number }}", + "pr-gate-comment-${{ github.event.pull_request.head.sha }}", ); + expect(workflow.concurrency?.group).toContain("github.event.pull_request.head.sha"); + expect(workflow.concurrency?.group).toContain("github.event.sha"); // Both comment-writing workflows share the group and neither cancels: // `cancel-in-progress: true` would kill an in-flight gate mutation when a // newer hygiene run starts, losing that read-modify-write. @@ -902,22 +905,18 @@ describe("GitHub Actions hardening", () => { expect(jobs.map(([name]) => name)).toEqual(["enforce-target"]); // The job is a runner plus steps, with one deliberate `if:` guard. The - // guard restricts the `issue_comment` trigger to maintainer comments on - // PRs — a comment on a plain issue, or from a non-maintainer, must not - // start this write-capable gate. On `pull_request_target` events the guard - // is always true, so it never disables the gate. + // guard accepts only successful CodeRabbit commit statuses and filters + // label events to the screenshot-waiver label. Other PR lifecycle events run. // No `permissions:` (a job-level block overrides the narrow workflow-level // one), no `container:`/`strategy:`/`outputs:`/`env:`/`defaults:`, and no // `<<:` merge key to reintroduce any of them sideways. const [, job] = jobs[0]!; expect(Object.keys(job).sort()).toEqual(["if", "runs-on", "steps"]); expect(job["runs-on"]).toBe("ubuntu-latest"); - expect(job["if"]).toContain("github.event_name != 'issue_comment'"); - expect(job["if"]).toContain("github.event.issue.pull_request != null"); - expect(job["if"]).toContain("coderabbitai[bot]"); - expect(job["if"]).toContain("'OWNER'"); - expect(job["if"]).toContain("'COLLABORATOR'"); - expect(job["if"]).toContain("'MEMBER'"); + expect(job["if"]).toContain("github.event_name == 'status'"); + expect(job["if"]).toContain("github.event.context == 'CodeRabbit'"); + expect(job["if"]).toContain("github.event.state == 'success'"); + expect(job["if"]).toContain("github.event.label.name == 'gui-screenshot-waived'"); // Checkout trusted scripts, then run the gate. Anything more is an extra // privileged action nobody reviewed. @@ -933,12 +932,11 @@ describe("GitHub Actions hardening", () => { "sparse-checkout", ]); expect(checkout.with).toEqual({ - // The event's base commit, not the repository default: pull_request_target - // runs this workflow from the base revision, and the scripts must match - // it — a merged gate would otherwise run against pre-promotion `main` - // scripts. The immutable SHA pins the checkout to the event's base commit. + // Normal PR events pin scripts to the event's base SHA. A trusted + // `status` event has no pull_request payload, so it loads scripts from + // the same default-branch trust boundary that owns the event. ref: - "${{ github.event_name == 'issue_comment' && github.event.repository.default_branch || github.event.pull_request.base.sha }}", + "${{ github.event_name == 'status' && github.event.repository.default_branch || github.event.pull_request.base.sha }}", "persist-credentials": false, // MAINTAINERS.md rides along so the completion ping reads the canonical // maintainer list from the same trusted base revision as the scripts. @@ -984,20 +982,18 @@ describe("GitHub Actions hardening", () => { const types = workflow.on?.pull_request_target?.types ?? []; expect([...types].sort()).toEqual([ "edited", + "labeled", "opened", "ready_for_review", "reopened", "synchronize", + "unlabeled", ]); - // A maintainer's GUI-waiver comment must re-run the gate. Issue comments - // are delivered as the `issue_comment` event, which is the only way the - // waiver can take effect without a PR edit or push. - expect(workflow.on?.issue_comment?.types).toBeDefined(); - expect([...(workflow.on?.issue_comment?.types ?? [])].sort()).toEqual([ - "created", - "edited", - ]); + // GUI-waiver labels re-run immediately through pull_request_target. + // CodeRabbit reviews re-run through the default-branch status event instead + // of using bot status-comment edits as workflow synchronisation. + expect(Object.prototype.hasOwnProperty.call(workflow.on ?? {}, "status")).toBe(true); // Review events must NOT be added: they load the workflow from the PR // head branch, breaking the base-pinned checkout (`pull_request_review` @@ -1023,13 +1019,12 @@ describe("GitHub Actions hardening", () => { // `Number(context.payload.pull_request.title)` — a value the PR author // controls, which turns the bot into a write primitive against any PR // number the author can name. Bind it to the immutable event field. - // `issue_comment` events carry the number under `issue`, so the resolution - // falls back from the PR object to the issue object — both are immutable - // event fields, never author-controlled title text. - expect(script).toMatch( - /const pull_number =\s*context\.payload\.pull_request\?\.number \?\?\s*context\.payload\.issue\?\.number;/, - ); - expect(script.match(/pull_number\s*=/g) ?? []).toHaveLength(1); + // Status runs resolve the immutable reviewed SHA back to exactly one open + // current-head PR before any write-capable operation. + expect(script).toContain("context.payload.pull_request?.number"); + expect(script).toContain("listPullRequestsAssociatedWithCommit"); + expect(script).toContain("candidate.head?.sha === statusSha"); + expect(script).toContain("candidates.length !== 1"); // Nothing may write back into the fetched PR. The audit round preserved the // required comparison line verbatim and defeated it one line earlier with @@ -1129,6 +1124,7 @@ describe("GitHub Actions hardening", () => { name !== "github.rest.pulls.listReviews" && name !== "github.rest.repos.getCollaboratorPermissionLevel" && name !== "github.rest.repos.compareCommitsWithBasehead" && + name !== "github.rest.repos.listPullRequestsAssociatedWithCommit" && // The claim check reads check-runs; it must never count as a write. name !== "github.rest.checks.listForRef", ); @@ -2659,12 +2655,7 @@ describe("GitHub Actions hardening", () => { expect(lastEnforcerCommentBody(result)).toContain("UI screenshot waived by a maintainer comment"); }); - test("an issue_comment event re-runs the gate and the waiver takes effect", async () => { - // This is the scenario that PR #1119 hit: a maintainer posts the waiver - // as an issue comment, and the gate must re-evaluate on that event — - // `pull_request_target` types do not include issue comments, so the - // separate `issue_comment` trigger carries it. The payload has no - // `pull_request` object; the PR number comes from `issue.number`. + test("the gui-screenshot-waived label clears the sole screenshot failure and reports the waiver", async () => { const result = await run({ pr: { base: { ref: "dev" }, @@ -2677,51 +2668,114 @@ describe("GitHub Actions hardening", () => { "- Ran bun test tests/ci-workflows.test.ts", ].join("\n"), }, - eventName: "issue_comment", - eventAction: "created", - comments: [ - { id: 1, user: { login: "wibias" }, author_association: "COLLABORATOR", body: "not touching gui" }, - ], + labels: ["gui-screenshot-waived"], }); expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); expect(lastEnforcerCommentBody(result)).not.toContain("UI screenshot required"); - expect(lastEnforcerCommentBody(result)).toContain("UI screenshot waived by a maintainer comment"); + expect(lastEnforcerCommentBody(result)).toContain("UI screenshot waived by the `gui-screenshot-waived` label"); }); - test("an issue_comment rerun does not accept a checklist with no recorded head", async () => { - // `issue_comment` events carry no `pull_request.head.sha`. A contributor - // who ticked the readiness checklist, then pushed, must not have that - // stale attestation accepted by a maintainer-waiver comment rerun — the - // gate must reset the boxes and re-draft. + + test("the gui-screenshot-waived label is reported after it clears the sole failure on the ready path", async () => { const result = await run({ pr: { base: { ref: "dev" }, + draft: true, title: "GUI: fix provider list spacing", body: readinessChecklistBody(4), }, - eventName: "issue_comment", - eventAction: "created", - comments: [ - { id: 1, user: { login: "wibias" }, author_association: "COLLABORATOR", body: "not touching gui" }, - readinessComment({ - version: 2, - autoDraftedByBot: false, - maintainersPinged: true, - completedAtHeadSha: null, - }), - ], + labels: ["gui-screenshot-waived"], maintainersFile: MAINTAINERS_FIXTURE, }); - // The comment-triggered rerun delivers no head SHA, so the completed - // checklist cannot be attributed to the live head: the gate resets the - // boxes and keeps the PR in draft. - const resetBody = callsTo(result, "pulls.update") as [{ body: string }]; - expect(resetBody[0]!.body).toContain(CHECKLIST_START); - expect(resetBody[0]!.body).not.toContain("- [x]"); - expect(resetBody[0]!.body).toContain("- [ ] All CI tests are green on my local testing."); - expect(resetBody[0]!.body).toContain("- [ ] My PR is ready for review."); + expect(result.warnings.some((w) => w.startsWith("setFailed:") && w.includes("screenshot"))).toBe(false); + expect(lastEnforcerCommentBody(result)).toContain("## ✅ READY"); + expect(lastEnforcerCommentBody(result)).not.toContain("UI screenshot required"); + expect(lastEnforcerCommentBody(result)).toContain("UI screenshot waived by the `gui-screenshot-waived` label"); + }); + + test("an ambiguous CodeRabbit status SHA fails closed before any PR mutation", async () => { + const headSha = "3f1c0de0a6a4d0a3f9a1b2c3d4e5f60718293a4b"; + const result = await run({ + pr: { base: { ref: "dev" } }, + eventName: "status", + statusSha: headSha, + associatedPullRequests: [ + { number: 42, state: "open", head: { sha: headSha } }, + { number: 77, state: "open", head: { sha: headSha } }, + ], + }); + + expect(methodsOf(result).filter(method => method !== "require")).toEqual(["repos.listPullRequestsAssociatedWithCommit"]); + expect(result.logs.join(" ")).toContain("maps to 2 open current-head PRs; skipping ambiguous/stale revalidation"); + expect(callsTo(result, "pulls.update")).toEqual([]); + expect(callsTo(result, "issues.createComment")).toEqual([]); + expect(callsTo(result, "issues.updateComment")).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(callsTo(result, "issues.removeLabel")).toEqual([]); + expect(callsTo(result, "graphql")).toEqual([]); + }); + + + test("a single open current-head PR resolves the CodeRabbit status SHA and runs the gate", async () => { + const headSha = "3f1c0de0a6a4d0a3f9a1b2c3d4e5f60718293a4b"; + const result = await run({ + pr: { base: { ref: "dev" }, number: 4242 }, + eventName: "status", + statusSha: headSha, + associatedPullRequests: [ + { number: 4242, state: "open", head: { sha: headSha } }, + ], + }); + + expect(callsTo(result, "pulls.get")).toEqual([ + { owner: "lidge-jun", repo: "opencodex", pull_number: 4242 }, + ]); + expect(result.logs.join(" ")).not.toContain("skipping ambiguous/stale revalidation"); + expect(methodsOf(result)).toContain("issues.listComments"); + }); + + test("CodeRabbit status association paginates before deciding uniqueness", async () => { + const headSha = "3f1c0de0a6a4d0a3f9a1b2c3d4e5f60718293a4b"; + const result = await run({ + pr: { base: { ref: "dev" } }, + eventName: "status", + statusSha: headSha, + associatedPullRequestPages: [ + [{ number: 42, state: "open", head: { sha: headSha } }], + [{ number: 77, state: "open", head: { sha: headSha } }], + ], + }); + + expect(callsTo(result, "repos.listPullRequestsAssociatedWithCommit")).toHaveLength(2); + expect(result.logs.join(" ")).toContain("maps to 2 open current-head PRs; skipping ambiguous/stale revalidation"); + expect(callsTo(result, "pulls.update")).toEqual([]); + expect(callsTo(result, "issues.createComment")).toEqual([]); + expect(callsTo(result, "issues.updateComment")).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(callsTo(result, "issues.removeLabel")).toEqual([]); + expect(callsTo(result, "graphql")).toEqual([]); + }); + + test("CodeRabbit status with no current-head match fails closed", async () => { + const statusSha = "3f1c0de0a6a4d0a3f9a1b2c3d4e5f60718293a4b"; + const result = await run({ + pr: { base: { ref: "dev" } }, + eventName: "status", + statusSha, + associatedPullRequests: [ + { number: 42, state: "open", head: { sha: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" } }, + ], + }); + + expect(result.logs.join(" ")).toContain("maps to 0 open current-head PRs; skipping ambiguous/stale revalidation"); + expect(callsTo(result, "pulls.update")).toEqual([]); + expect(callsTo(result, "issues.createComment")).toEqual([]); + expect(callsTo(result, "issues.updateComment")).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(callsTo(result, "issues.removeLabel")).toEqual([]); + expect(callsTo(result, "graphql")).toEqual([]); }); test("a non-maintainer issue_comment does not re-run the gate", async () => { diff --git a/tests/helpers/enforce-pr-target-harness.ts b/tests/helpers/enforce-pr-target-harness.ts index 9f4710a21..90a376de8 100644 --- a/tests/helpers/enforce-pr-target-harness.ts +++ b/tests/helpers/enforce-pr-target-harness.ts @@ -83,12 +83,21 @@ export type RunOptions = { */ eventAction?: string; /** - * Webhook event name. Defaults to `"pull_request_target"`. Pass - * `"issue_comment"` to exercise the GUI-waiver re-run path: the payload then - * carries `issue` and `comment` (never `pull_request`), exactly as GitHub - * delivers an issue comment on a PR. + * Webhook event name. Defaults to `"pull_request_target"`. `issue_comment` + * remains available for fail-closed compatibility tests; `status` models the + * default-branch CodeRabbit wake-up path. */ eventName?: string; + /** SHA carried by a `status` event. Defaults to the live PR head SHA. */ + statusSha?: string; + /** Legacy commit-status context. Defaults to `CodeRabbit`. */ + statusContext?: string; + /** Legacy commit-status state. Defaults to `success`. */ + statusState?: string; + /** Shorthand for a single associated-PR response page. */ + associatedPullRequests?: unknown[]; + /** Page-specific PRs returned by repos.listPullRequestsAssociatedWithCommit. */ + associatedPullRequestPages?: unknown[][]; /** * `author_association` of the commenter on an `issue_comment` event. * Defaults to `"COLLABORATOR"`. The gate only re-runs for maintainer @@ -581,7 +590,14 @@ export async function runEnforcePrTarget( const openPullPages: unknown[][] = options.openPullPages ?? (options.openPulls && options.openPulls.length > 0 ? [options.openPulls] : []); - const paginatePageCount = Math.max(pages.length, openPullPages.length, 1); + const associatedPullRequestPages: unknown[][] = + options.associatedPullRequestPages ?? [options.associatedPullRequests ?? [pr]]; + const paginatePageCount = Math.max( + pages.length, + openPullPages.length, + associatedPullRequestPages.length, + 1, + ); /** * Record the call, then either reject or return a plausible payload. Every @@ -706,6 +722,14 @@ export async function runEnforcePrTarget( const basehead = String((args as { basehead?: string })?.basehead ?? ""); return respond("repos.compareCommitsWithBasehead", args, compareResult(basehead)); }, + listPullRequestsAssociatedWithCommit: (args: unknown) => { + const page = Number((args as { page?: number })?.page ?? 1); + return respond( + "repos.listPullRequestsAssociatedWithCommit", + args, + associatedPullRequestPages[page - 1] ?? [], + ); + }, }, }; @@ -763,8 +787,8 @@ export async function runEnforcePrTarget( respond("request", { route, params }); /** * `github.paginate(fn, params)` — walk every page and concatenate, the way - * Octokit does. Page count covers both comment and open-PR fixtures so a - * stacked parent on page two is still visible. + * Octokit does. Page count covers comment, open-PR, and associated-PR + * fixtures so a relevant record on page two is still visible. */ paginate = Object.assign( async (fn: (args: unknown) => Promise<{ data: unknown[] }>, params: unknown) => { @@ -870,7 +894,13 @@ export async function runEnforcePrTarget( author_association: options.commentAuthorAssociation ?? "COLLABORATOR", }, } - : { pull_request: eventPr }), + : options.eventName === "status" + ? { + sha: options.statusSha ?? pr.head.sha, + context: options.statusContext ?? "CodeRabbit", + state: options.statusState ?? "success", + } + : { pull_request: eventPr }), repository: { id: 987654321, name: "opencodex", diff --git a/tests/zz-pr-coderabbit-readiness-revalidation.test.ts b/tests/zz-pr-coderabbit-readiness-revalidation.test.ts index e65183cb8..bab6f22af 100644 --- a/tests/zz-pr-coderabbit-readiness-revalidation.test.ts +++ b/tests/zz-pr-coderabbit-readiness-revalidation.test.ts @@ -1,54 +1,101 @@ import { describe, expect, test } from "bun:test"; +type WorkflowJob = { + if?: string; + "runs-on"?: string; + steps?: Array<{ + name?: string; + uses?: string; + run?: string; + with?: Record; + }>; +}; + type Workflow = { on?: { issue_comment?: { types?: string[] }; + pull_request_target?: { types?: string[] }; + pull_request_review?: { types?: string[] }; + workflow_run?: { workflows?: string[]; types?: string[] }; + status?: unknown; }; - jobs?: Record< - string, - { - if?: string; - steps?: Array<{ - name?: string; - with?: Record; - }>; - } - >; + jobs?: Record; }; -describe("CodeRabbit readiness revalidation", () => { - test("CodeRabbit PR status comments can rerun the findings gate", async () => { +describe("workflow comment-spam hardening", () => { + test("PR gate consumes CodeRabbit commit status from the trusted default branch", 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"]); + expect(workflow.on?.issue_comment).toBeUndefined(); + expect(workflow.on?.pull_request_review).toBeUndefined(); + expect(workflow.on?.workflow_run).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(workflow.on ?? {}, "status")).toBe(true); + expect(workflow.on?.pull_request_target?.types).toEqual(expect.arrayContaining([ + "edited", + "labeled", + "ready_for_review", + "synchronize", + "unlabeled", + ])); 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 normalize = (value: string | undefined) => + (value ?? "").replace(/\s+/g, " ").trim(); + expect(normalize(job?.if)).toBe(normalize(` + (github.event_name == 'status' && + github.event.context == 'CodeRabbit' && + github.event.state == 'success') || + (github.event_name == 'pull_request_target' && + ((github.event.action != 'labeled' && github.event.action != 'unlabeled') || + github.event.label.name == 'gui-screenshot-waived')) + `)); const checkoutStep = job?.steps?.find( step => step.name === "Checkout trusted PR-quality scripts", ); expect(checkoutStep?.with?.ref).toBe( - "${{ github.event_name == 'issue_comment' && github.event.repository.default_branch || github.event.pull_request.base.sha }}", + "${{ github.event_name == 'status' && github.event.repository.default_branch || github.event.pull_request.base.sha }}", ); 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("github.paginate"); + expect(script).toContain("listPullRequestsAssociatedWithCommit"); + expect(script).toContain('candidate.state === "open"'); + expect(script).toContain("candidate.head?.sha === statusSha"); + expect(script).toContain("candidates.length !== 1"); + expect(script).toContain('context.eventName === "status"'); + expect(script).toContain('const GUI_SCREENSHOT_WAIVER_LABEL = "gui-screenshot-waived"'); + expect(script).toContain("screenshotWaiverNotice"); expect(script).toContain("unresolvedFindingsClaim"); }); + + test("issue-comment translation rejects PR and bot comments before runner allocation", async () => { + const text = await Bun.file( + new URL("../.github/workflows/enforce-issue-quality.yml", import.meta.url), + ).text(); + const workflow = Bun.YAML.parse(text) as Workflow; + const jobIf = workflow.jobs?.["translate-comment"]?.if ?? ""; + + expect(jobIf).toContain("github.event_name == 'issue_comment'"); + expect(jobIf).toContain("github.event.issue.pull_request == null"); + expect(jobIf).toContain("github.event.comment.user.type != 'Bot'"); + }); + + test("contributor docs describe the label waiver and commit-status trust boundary", async () => { + const docs = await Bun.file( + new URL("../docs-site/src/content/docs/contributing/pr-quality.md", import.meta.url), + ).text(); + + expect(docs).toContain("gui-screenshot-waived"); + expect(docs).toContain("`CodeRabbit` commit status"); + expect(docs).toContain("`status` event"); + expect(docs).toContain("exactly one open"); + expect(docs).toContain("CodeRabbit status-comment edits do not trigger the PR gate"); + }); });