Skip to content

fix(review-pr): remove auto-review, require explicit reviewer request only #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
derekmisler wants to merge 1 commit into from
+96 −59
Closed

fix(review-pr): remove auto-review, require explicit reviewer request only #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 17 additions & 30 deletions .agents/skills/add-pr-reviewer-to-repo/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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 `<!-- docker-agent-review -->` / `<!-- docker-agent-review-reply -->` 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-<repo>-<pr>-*`) prevents duplicate reviews from racing on the same PR. |
Expand Down Expand Up @@ -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.

---

Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
59 changes: 59 additions & 0 deletions .github/workflows/pr-review-trigger.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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, '<!-- docker-agent-review -->') &&
!contains(github.event.comment.body, '<!-- docker-agent-review-reply -->') &&
!contains(github.event.comment.body, '<!-- cagent-review -->') &&
!contains(github.event.comment.body, '<!-- cagent-review-reply -->')
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
4 changes: 2 additions & 2 deletions .github/workflows/review-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ jobs:
!contains(github.event.comment.body, '<!-- docker-agent-review-reply -->') &&
!contains(github.event.comment.body, '<!-- cagent-review -->') &&
!contains(github.event.comment.body, '<!-- cagent-review-reply -->')) ||
(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')
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
45 changes: 18 additions & 27 deletions review-pr/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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).
Expand All @@ -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
Expand All @@ -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 `<!-- docker-agent-review -->`/`<!-- docker-agent-review-reply -->` 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-<repo>-<pr>-*`) prevents duplicate reviews from racing on the same PR. |
Expand Down
Loading