Skip to content

ci: move concurrency dedup from trigger to review workflow #2789

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

Merged
dgageot merged 2 commits into from
May 13, 2026
+17 −5
Merged

ci: move concurrency dedup from trigger to review workflow #2789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc085cc
ci: move concurrency dedup from trigger to review workflow
dgageot May 13, 2026
b38515a
fix: exempt pr-review-trigger.yml from concurrency check
dgageot May 13, 2026
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
4 changes: 0 additions & 4 deletions .github/workflows/pr-review-trigger.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ on:
pull_request_review_comment:
types: [created]

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

permissions: {}

jobs:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/pr-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ on:
workflows: ["PR Review - Trigger"]
types: [completed]

concurrency:
group: pr-review-${{ github.event.issue.number || github.event.workflow_run.pull_requests[0].number || github.run_id }}
Copy link
Copy Markdown

@docker-agent docker-agent May 13, 2026

Choose a reason for hiding this comment

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

[MEDIUM] Fork PR workflow_run events fall back to run_id, defeating deduplication

For fork PRs — which are the primary use-case for the trigger-based workflow_run path — GitHub deliberately leaves workflow_run.pull_requests as an empty array for security reasons. As a result, github.event.workflow_run.pull_requests[0].number evaluates to empty in a GitHub Actions expression, and || falls through to github.run_id.

Because run_id is unique per run, every fork-PR review run lands in its own concurrency group, making cancel-in-progress: true completely ineffective for fork PRs. If opened + review_requested fire back-to-back for a fork PR (the exact scenario this PR aims to fix), both pr-review.yml runs will proceed independently instead of the first being cancelled.

Suggested fix: extract the PR number earlier in the workflow — e.g., store it in the context artifact during save-context and pass it as a workflow input — or use a jq expression on the artifact file to derive the group key.

Alternatively, a workaround for the workflow_run path is to parse the PR number out of the triggering workflow's inputs/artifacts rather than relying on workflow_run.pull_requests.

Copy link
Copy Markdown

@docker-agent docker-agent May 13, 2026

Choose a reason for hiding this comment

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

[LOW] Issue numbers and PR numbers share the same concurrency group key namespace

GitHub issues and PRs share a single number sequence per repository. An issue_comment event on a plain issue #42 will produce group key pr-review-42, which is identical to the group key a workflow_run triggered by PR #42 would produce.

The workflow-level concurrency block is evaluated before any job-level if: conditions, so a workflow_run for PR #42 could cancel a concurrently-running issue_comment-triggered workflow for issue #42 (or vice versa) even when they are unrelated. In practice the review job's if: condition prevents any work from happening for non-PR issue comments, but the cancellation of the competing workflow run is still visible in the checks list.

Suggestion: Distinguish event sources in the group key, e.g.:

group: pr-review-${{ github.event_name }}-${{ github.event.issue.number || github.event.workflow_run.pull_requests[0].number || github.run_id }}

This limits any cancellation to runs of the same event type, though the fork-PR limitation still applies to workflow_run events.

cancel-in-progress: true

permissions:
contents: read # Required at top-level to give `issue_comment` events access to the secrets below.

Expand Down
14 changes: 13 additions & 1 deletion scripts/workflow-lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#
# 1. concurrency: every PR-triggered workflow declares a
# concurrency: group (AGENTS.md §
# GitHub Actions);
# GitHub Actions), EXCEPT pr-review-trigger.yml
# which intentionally runs all events to completion
# (see PR #2789);
#
# 2. pinned-by-sha: every third-party `uses:` reference is
# pinned by a 40-char SHA, not a tag/branch
Expand Down Expand Up @@ -64,8 +66,18 @@ note() {
# yq-based so it runs in the lint job without extra deps; the
# trade-off is a false positive if `pull_request` appears in a
# comment, which we accept.
#
# EXCEPTION: pr-review-trigger.yml is exempt from this check because
# it intentionally runs all events to completion (the workflow is cheap,
# and deduplication happens downstream in pr-review.yml). See PR #2789.
for f in "$WORKFLOWS_DIR"/*.yml "$WORKFLOWS_DIR"/*.yaml; do
[ -e "$f" ] || continue

# Skip pr-review-trigger.yml
if [[ "$f" == *"pr-review-trigger.yml" ]]; then
continue
fi

if grep -qE '\bpull_request\b' "$f"; then
if ! grep -qE '^\s*concurrency:' "$f"; then
note "$f" "PR-triggered workflow has no concurrency: block (AGENTS.md § GitHub Actions)"
Expand Down
Loading