Skip to content

ci: add concurrency group to pr-review-trigger to prevent duplicate reviews#13890

Open
derekmisler wants to merge 1 commit into
docker:mainfrom
derekmisler:fix/pr-review-trigger-concurrency
Open

ci: add concurrency group to pr-review-trigger to prevent duplicate reviews#13890
derekmisler wants to merge 1 commit into
docker:mainfrom
derekmisler:fix/pr-review-trigger-concurrency

Conversation

@derekmisler

Copy link
Copy Markdown
Contributor

Adds a concurrency group keyed by PR number to pr-review-trigger.yml to prevent
triplicate reviews when simultaneous pull_request events fire for the same fork PR
(e.g., multiple review_requested events when several reviewers are added at once).

Also skips Bot-type senders (Dependabot, Renovate) early to save Actions minutes.

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The concurrency group and bot-skip condition added in this PR are correct.

Concurrency group (line 15): Uses github.event.pull_request.number as the key. Both pull_request and pull_request_review_comment events include a pull_request object in their payload (unlike issue_comment), so the group key is always well-formed and will correctly deduplicate simultaneous events for the same PR without collapsing across unrelated PRs.

Bot-sender skip (line 21): github.event.sender.type != 'Bot' uses the correct capitalization ('Bot'), which matches GitHub's documented sender type values. Combined with the fork-repo guard (github.event.pull_request.head.repo.fork), the condition correctly gates execution to fork PRs from non-bot actors.

No bugs introduced by this PR were found.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…eviews

Signed-off-by: Derek Misler <derek.misler@docker.com>
@derekmisler derekmisler force-pushed the fix/pr-review-trigger-concurrency branch from a278523 to a30b245 Compare June 30, 2026 16:20
@derekmisler derekmisler marked this pull request as ready for review June 30, 2026 16:32
@derekmisler derekmisler requested a review from a team as a code owner June 30, 2026 16:32

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The PR correctly adds a concurrency group to deduplicate simultaneous pull_request events for fork PRs, and gates the job on fork == true plus a bot-sender check. No high- or medium-severity bugs were introduced.

Minor observations (not blocking):

  • The concurrency key pr-review-trigger-${{ github.event.pull_request.number }} would collapse to pr-review-trigger- if the PR number were ever absent (e.g., if the on: block were later extended to non-pull_request events). Consider including ${{ github.event_name }} in the key for robustness.
  • The github.event.pull_request.head.repo.fork boolean check silently skips same-repo PRs — this is intentional but may benefit from a comment in the YAML for future maintainers.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the GitHub Actions trigger workflow used to kick off PR reviews, reducing duplicate downstream review runs caused by multiple near-simultaneous pull_request events on the same PR, and avoiding unnecessary executions for bot-originated events.

Changes:

  • Added a workflow-level concurrency group keyed by PR number to deduplicate concurrent runs.
  • Added an early job if: guard to run only for fork PRs and to skip bot senders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants