From e7390499936f166d04e86f8eb57ce907cb551907 Mon Sep 17 00:00:00 2001 From: Derek Misler Date: Tue, 30 Jun 2026 12:53:38 +0000 Subject: [PATCH] fix(review-pr): remove auto-review, require explicit reviewer request only Reviews now only run when docker-agent is explicitly requested as a reviewer (review_requested). The previous auto-review on opened/ready_for_review is removed across all templates, docs, and the reusable workflow. Changes: - .github/workflows/pr-review-trigger.yml: new canonical fork-PR trigger template with only review_requested, concurrency group, fork-only if condition (head.repo.fork && sender.type != 'Bot'), and bot-actor skip - .github/workflows/review-pr.yml: remove the auto-review pull_request branch (action != review_requested) from the review job if condition; update trigger-type classification comment - review-pr/README.md: remove all auto-review language; update workflow snippets, trigger table, flow diagram, and defense-in-depth section - .agents/skills/add-pr-reviewer-to-repo/SKILL.md: remove trigger mode section (Modes A/B), update all YAML examples and audit checklist to review_requested only; update fork trigger template with concurrency group and if condition --- .../skills/add-pr-reviewer-to-repo/SKILL.md | 47 ++++++--------- .github/workflows/pr-review-trigger.yml | 59 +++++++++++++++++++ .github/workflows/review-pr.yml | 4 +- review-pr/README.md | 45 ++++++-------- 4 files changed, 96 insertions(+), 59 deletions(-) create mode 100644 .github/workflows/pr-review-trigger.yml diff --git a/.agents/skills/add-pr-reviewer-to-repo/SKILL.md b/.agents/skills/add-pr-reviewer-to-repo/SKILL.md index d25c22b..4b53f8d 100644 --- a/.agents/skills/add-pr-reviewer-to-repo/SKILL.md +++ b/.agents/skills/add-pr-reviewer-to-repo/SKILL.md @@ -18,7 +18,7 @@ Use this skill when you are asked to add AI-powered PR review to a repo, or to u > > **`/review` comment:** still works but is deprecated. Prefer the sidebar workflow. > -> **External / fork contributors:** auto-review only runs on org members' own PRs. To review an external contributor's PR, an org member requests `docker-agent` as a reviewer — the review is authorized by the **requester**, not the PR author. +> **External / fork contributors:** reviews only run when someone explicitly requests `docker-agent` as a reviewer. There is no auto-review on PR open or ready-for-review. To review an external contributor's PR, an org member requests `docker-agent` as a reviewer — the review is authorized by the **requester**, not the PR author. Make sure to communicate this to contributors when onboarding a repo — it's the main daily interaction pattern and easy to miss if someone only reads the workflow YAML. @@ -31,7 +31,7 @@ The reusable workflow handles all of the following internally. **Do not add call | Concern | How it's handled internally | | ------- | --------------------------- | | **Bot comment filtering** | All jobs in the reusable workflow carry comprehensive `if:` conditions that skip `docker-agent`, `docker-agent[bot]`, any `Bot`-type user, and comments containing `` / `` HTML markers. | -| **Org membership / authorization** | A dedicated `check-org-membership` step runs before any review work begins. Auto-review verifies the **PR author**; a requested review verifies the **requester** (so a maintainer can pull an external contributor's PR into review); comment paths verify the commenter. Callers never need their own `author_association` checks. | +| **Org membership / authorization** | A dedicated `check-org-membership` step runs before any review work begins. A requested review verifies the **requester** (so a maintainer can pull an external contributor's PR into review); comment paths verify the commenter. Callers never need their own `author_association` checks. | | **PR vs issue comment disambiguation** | The reusable workflow checks `github.event.issue.pull_request` internally. Plain issue comments on non-PR issues are ignored automatically. | | **Draft PR skipping** | Handled internally — draft PRs are not reviewed. | | **Concurrent review guard** | A cache-based lock (`pr-review-lock---*`) prevents duplicate reviews from racing on the same PR. | @@ -71,27 +71,9 @@ Use `@main` only for bleeding-edge / pre-release testing. --- -## 3. Choose a Trigger Mode +## 3. Trigger Mode -The `pull_request` event types control how often reviews run. Pick one mode and apply it to the trigger section of the workflow(s) below. - -**Mode B — recommended default** (reviews on open, ready, and explicit re-request only): - -```yaml -pull_request: - types: [opened, ready_for_review, review_requested] -``` - -**Mode A — continuous re-review on every push** (adds `synchronize`): - -```yaml -pull_request: - types: [opened, ready_for_review, synchronize, review_requested] -``` - -Mode A costs more workflow minutes. Opt in only if the team wants the reviewer to automatically re-examine every push to the PR branch. - -The examples below use **Mode B**. +Reviews are only triggered by explicitly requesting `docker-agent` as a reviewer (`review_requested`). There is no auto-review on PR open or ready-for-review. The examples below use `review_requested` only. --- @@ -103,7 +85,7 @@ Create one file: **`.github/workflows/pr-review.yml`** name: PR Review on: pull_request: - types: [ready_for_review, opened, review_requested] + types: [review_requested] issue_comment: types: [created] pull_request_review_comment: @@ -142,14 +124,21 @@ Lightweight — no secrets needed, runs in the fork's context: name: PR Review - Trigger on: pull_request: - types: [ready_for_review, opened, review_requested] + types: [review_requested] pull_request_review_comment: types: [created] permissions: {} +# Deduplicate simultaneous pull_request events for the same fork PR. +# Only the last trigger completes, producing a single workflow_run → one review. +concurrency: + group: pr-review-trigger-${{ github.event.pull_request.number || github.run_id }}-${{ github.event.comment.id || 'review-request' }} + cancel-in-progress: true + jobs: save-context: + if: github.event.pull_request.head.repo.fork && github.event.sender.type != 'Bot' runs-on: ubuntu-latest steps: - name: Save event context @@ -217,7 +206,7 @@ Replace `@VERSION` in the `uses:` line with the tag from Step 2 (e.g. `@v2.0.0`) ### How the two workflows interact ``` -pull_request (opened / ready_for_review / review_requested) +pull_request (review_requested) → pr-review-trigger.yml (saves context artifact, no secrets needed) → completes → workflow_run fires @@ -292,9 +281,9 @@ jobs: ### Reviews don't run on fork PRs at all -**Cause:** The trigger workflow (`pr-review-trigger.yml`) is missing, or its `pull_request` trigger types don't include `opened` / `ready_for_review` / `review_requested`. +**Cause:** The trigger workflow (`pr-review-trigger.yml`) is missing, or its `pull_request` trigger types don't include `review_requested`. -**Fix:** Confirm `pr-review-trigger.yml` exists in `.github/workflows/` on the default branch and that its `on.pull_request.types` list matches the desired trigger mode. +**Fix:** Confirm `pr-review-trigger.yml` exists in `.github/workflows/` on the default branch and that its `on.pull_request.types` includes `review_requested`. ### Review doesn't appear as a check run @@ -386,9 +375,7 @@ Check the `permissions:` block on the `review` job in `pr-review.yml`: Check `on.pull_request.types` in `pr-review.yml` (or `pr-review-trigger.yml` for fork setups): -- [ ] Includes `review_requested` — the **primary trigger** (sidebar reviewer UX) -- [ ] Includes `ready_for_review` -- [ ] Includes `opened` +- [ ] Includes `review_requested` — the **only supported trigger** (sidebar reviewer UX) #### Unnecessary caller-side `if:` guards diff --git a/.github/workflows/pr-review-trigger.yml b/.github/workflows/pr-review-trigger.yml new file mode 100644 index 0000000..87859af --- /dev/null +++ b/.github/workflows/pr-review-trigger.yml @@ -0,0 +1,59 @@ +# Copyright The Docker Agent Action authors +# SPDX-License-Identifier: Apache-2.0 + +name: PR Review - Trigger +on: + pull_request: + types: [review_requested] + pull_request_review_comment: + types: [created] + +permissions: {} + +# Deduplicate simultaneous pull_request events for the same fork PR +# (e.g. multiple review_requested events firing at once for the same PR). +# Only the last trigger completes, producing a single workflow_run → one review. +# Distinct review comments stay independent (the comment id keys them into their +# own group) so a real reply is never dropped by another comment on the same PR. +concurrency: + group: pr-review-trigger-${{ github.event.pull_request.number || github.run_id }}-${{ github.event.comment.id || 'review-request' }} + cancel-in-progress: true + +jobs: + save-context: + # Only run on fork PRs (same-repo PRs have full OIDC access and don't need + # the trigger-artifact path). Skip Bot-account senders (Dependabot, Renovate, + # etc.) early to save Actions minutes. Note: docker-agent is a User account, + # not a Bot, so it is not filtered here. + if: > + github.event.pull_request.head.repo.fork && + github.event.sender.type != 'Bot' && + github.event.comment.user.login != 'docker-agent' && + github.event.comment.user.login != 'docker-agent[bot]' && + github.event.comment.user.type != 'Bot' && + !contains(github.event.comment.body, '') && + !contains(github.event.comment.body, '') && + !contains(github.event.comment.body, '') && + !contains(github.event.comment.body, '') + runs-on: ubuntu-latest + steps: + - name: Save event context + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + COMMENT_JSON: ${{ toJSON(github.event.comment) }} + run: | + mkdir -p context + printf '%s' "${{ github.event_name }}" > context/event_name.txt + printf '%s' "$PR_NUMBER" > context/pr_number.txt + printf '%s' "$PR_HEAD_SHA" > context/pr_head_sha.txt + if [ "${{ github.event_name }}" = "pull_request_review_comment" ]; then + printf '%s' "$COMMENT_JSON" > context/comment.json + fi + + - name: Upload context + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: pr-review-context + path: context/ + retention-days: 1 diff --git a/.github/workflows/review-pr.yml b/.github/workflows/review-pr.yml index acfaadb..65e9d75 100644 --- a/.github/workflows/review-pr.yml +++ b/.github/workflows/review-pr.yml @@ -177,7 +177,6 @@ jobs: !contains(github.event.comment.body, '') && !contains(github.event.comment.body, '') && !contains(github.event.comment.body, '')) || - (github.event_name == 'pull_request' && github.event.action != 'review_requested' && github.event.sender.type != 'Bot' && github.event.sender.login != 'docker-agent' && github.event.sender.login != 'docker-agent[bot]') || (github.event_name == 'pull_request' && github.event.action == 'review_requested' && github.event.requested_reviewer.login == 'docker-agent') || inputs.pr-number != '' || (needs.resolve-context.result == 'success' && needs.resolve-context.outputs.trigger-event == 'pull_request') @@ -271,6 +270,7 @@ jobs: IS_REVIEW_COMMAND: ${{ steps.command.outputs.is_review }} run: | # Classify whether this is a user-requested or automatic review. + # All pull_request-triggered reviews are now user-requested (review_requested only). # User-requested triggers bypass the draft check only; org-membership # check still applies on all paths via the downstream step. if [ "$EVENT_NAME" = "issue_comment" ] && [ "$IS_REVIEW_COMMAND" = "true" ]; then @@ -287,7 +287,7 @@ jobs: echo "🎯 Trigger type: user-requested (trigger path via resolve-context)" else echo "user_requested=false" >> $GITHUB_OUTPUT - echo "🤖 Trigger type: automatic (pull_request event)" + echo "🤖 Trigger type: fallback (unclassified event)" fi - name: Check if PR is draft diff --git a/review-pr/README.md b/review-pr/README.md index bbe4ee7..3a913b5 100644 --- a/review-pr/README.md +++ b/review-pr/README.md @@ -2,7 +2,7 @@ AI-powered pull request review using a multi-agent system. Analyzes code changes, posts inline comments, and learns from your feedback. -> **Primary trigger:** Add `docker-agent` as a reviewer in the PR sidebar — the review starts automatically. To re-trigger a review, re-request a review from `docker-agent` in the PR sidebar. The `/review` comment still works but is deprecated. +> **Primary trigger:** Add `docker-agent` as a reviewer in the PR sidebar — the review starts automatically. To re-trigger a review, re-request a review from `docker-agent` in the PR sidebar. ## Quick Start @@ -16,7 +16,7 @@ If your repo only accepts PRs from branches within the same repo (no forks), you name: PR Review on: pull_request: - types: [ready_for_review, opened, review_requested] + types: [review_requested] issue_comment: types: [created] pull_request_review_comment: @@ -49,14 +49,24 @@ Fork PRs are subject to GitHub's security restrictions: `pull_request` and `pull name: PR Review - Trigger on: pull_request: - types: [ready_for_review, opened, review_requested] + types: [review_requested] pull_request_review_comment: types: [created] permissions: {} +# Deduplicate simultaneous pull_request events for the same fork PR +# (e.g. multiple review_requested events firing at once for the same PR). +# Only the last trigger completes, producing a single workflow_run → one review. +# Distinct review comments stay independent (the comment id keys them into their +# own group) so a real reply is never dropped by another comment on the same PR. +concurrency: + group: pr-review-trigger-${{ github.event.pull_request.number || github.run_id }}-${{ github.event.comment.id || 'review-request' }} + cancel-in-progress: true + jobs: save-context: + if: github.event.pull_request.head.repo.fork && github.event.sender.type != 'Bot' runs-on: ubuntu-latest steps: - name: Save event context @@ -120,7 +130,7 @@ jobs: #### How the two workflows interact ``` -pull_request (opened / ready_for_review / review_requested) +pull_request (review_requested) → pr-review-trigger.yml (saves context as artifact, no secrets needed) → completes → workflow_run fires @@ -138,30 +148,12 @@ pull_request_review_comment Adding `docker-agent` as a reviewer fires a `pull_request` event with `action: review_requested`, which follows the trigger-workflow path above. The `issue_comment` event (`/review` command and `@docker-agent` mentions) always has full permissions regardless of fork status, so those paths work directly without the trigger workflow. -### Choosing a trigger mode - -The `pull_request` trigger types in your calling workflow control how often reviews run. Two modes are supported — the examples above use **Mode B**: - -**Mode B — recommended default:** -```yaml -pull_request: - types: [opened, ready_for_review, review_requested] -``` -Reviews run when a PR is opened or marked ready for review. After the initial review, further `pull_request`-triggered reviews only run when `docker-agent` is explicitly re-requested as a reviewer. Re-request a review from `docker-agent` in the PR sidebar to re-trigger at any time. The `/review` comment still works but is deprecated. - -**Mode A — continuous re-review on every push:** -```yaml -pull_request: - types: [opened, ready_for_review, synchronize, review_requested] -``` -Adds `synchronize` to also trigger on every push to the PR branch. Opt in if your team wants the reviewer to automatically re-examine every update, at the cost of more workflow runs. - ### External and fork contributor PRs > [!NOTE] > The requester-authorized path below requires the `check-org-membership` update from PR #16 (merge that PR first). Until it ships, membership is checked against the PR author rather than the requesting org member, so requesting `docker-agent` on an external or fork PR is silently skipped. -Auto-review only runs on PRs authored by org members. A PR opened by an external or fork contributor is **not** reviewed automatically. To get one reviewed, an org member drives it through GitHub's native UI in two steps: +Reviews are exclusively triggered by explicitly requesting `docker-agent` as a reviewer. A PR opened by an external or fork contributor is not reviewed automatically. To get one reviewed, an org member drives it through GitHub's native UI in two steps: 1. **Approve the workflow run.** For PRs from first-time and external contributors, GitHub holds all Actions runs until a maintainer approves them (governed by the repository's `Settings` → `Actions` → `General` fork-PR approval policy). Click **Approve and run workflows** on the PR; until then nothing runs, including the PR review trigger. 2. **Request a review from `docker-agent`.** In the PR sidebar, under **Reviewers**, add `docker-agent`. This fires a `review_requested` event and starts the review, shown as a check run (if `checks: write` is granted). @@ -182,14 +174,13 @@ with: | Trigger | Behavior | | ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------- | | Request review from `docker-agent` | **Primary trigger.** Add `docker-agent` as a reviewer in the PR sidebar — review starts automatically, shown as a check run. Authorized by the requesting org member, so it also works for external/fork contributors' PRs. | -| PR opened/ready | Auto-reviews when a PR is opened or marked ready for review (org-member-authored PRs). | -| ~~`/review`~~ _(deprecated)_ | Re-trigger a review, or trigger manually when auto-review hasn't run (e.g. after a force-push). Shows as a check run if `checks: write` is granted. | +| ~~`/review`~~ _(deprecated)_ | Re-trigger a review. Shows as a check run if `checks: write` is granted. | | Reply to review comment | Responds in-thread and captures feedback to improve future reviews. | | `@docker-agent` mention | Answers questions and clarifies review findings. Works in both PR-level issue comments and inline file-line review comments, including on fork PRs (via the trigger workflow). | > **Built-in defense-in-depth:** > -> 1. **Verifies org membership** before every review. Auto-review checks the **PR author** (so only org members' PRs are reviewed automatically); a **requested review** checks the **requester**, so a maintainer can pull an external contributor's PR into review on demand; `/review` checks the commenter +> 1. **Verifies org membership** before every review. A **requested review** checks the **requester**, so a maintainer can pull an external contributor's PR into review on demand; `/review` checks the commenter > 2. **Prevents bot cascades** — replies from bots (except `docker-agent`) are ignored > 3. **Throttles rate anomalies** — per-PR `concurrency:` groups collapse same-trigger bursts, and a rate-limit check skips the review when too many requests land on one PR in a short window > 4. **Fork PRs work automatically** with the two-workflow setup — the trigger → `workflow_run` pattern provides OIDC/secret access regardless of fork status @@ -201,7 +192,7 @@ The workflow YAML examples above are the complete, recommended setup. The reusab | Protection | How it's handled | | ---------- | ---------------- | | **Bot comment filtering** | All jobs in the reusable workflow filter out `docker-agent`, `docker-agent[bot]`, any `Bot`-type user, and comments with ``/`` markers. No caller-side filtering needed. | -| **Org membership / authorization** | A `check-org-membership` step runs before any review work. Auto-review verifies the **PR author**; a requested review verifies the **requester** (so an external contributor's PR can be reviewed when an org member requests it); comment / `/review` paths verify the commenter. All via OIDC. Callers never need `author_association` checks. | +| **Org membership / authorization** | A `check-org-membership` step runs before any review work. A requested review verifies the **requester** (so an external contributor's PR can be reviewed when an org member requests it); comment / `/review` paths verify the commenter. All via OIDC. Callers never need `author_association` checks. | | **PR vs issue comment** | The reusable workflow checks `github.event.issue.pull_request` internally. Plain issue comments on non-PR issues are silently ignored. | | **Draft PR skipping** | Draft PRs are skipped internally — no caller condition needed. | | **Concurrent review guard** | A cache-based lock (`pr-review-lock---*`) prevents duplicate reviews from racing on the same PR. |