From 3fc126030e4e11708608333ef5278498eba631e1 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:22:59 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20implement=20issue=20#1126=20?= =?UTF-8?q?=E2=80=94=20Free-plan=2020-job=20cap:=20PR=20Auto-Review=20Read?= =?UTF-8?q?y=20Check=20has=20no=20concurrency=20and=20fans=20out=20per=20c?= =?UTF-8?q?heck=5Fsuite/workflow=5Frun=20=E2=80=94=20dedupe=20the=20defaul?= =?UTF-8?q?t-branch-context=20runs=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/pr-auto-review.yml | 23 +++ standards/workflows/pr-auto-review.yml | 23 +++ .../workflows/pr-auto-review/concurrency.bats | 148 ++++++++++++++++++ 3 files changed, 194 insertions(+) create mode 100644 test/workflows/pr-auto-review/concurrency.bats diff --git a/.github/workflows/pr-auto-review.yml b/.github/workflows/pr-auto-review.yml index 7d132dc32..05062864a 100644 --- a/.github/workflows/pr-auto-review.yml +++ b/.github/workflows/pr-auto-review.yml @@ -41,6 +41,29 @@ on: permissions: {} +# Deduplicate ONLY the default-branch-context triggers (issue #1126). On GitHub +# Free (20 concurrent jobs, org-wide) this stub fanned out per trigger. Its runs +# split by context: +# • check_suite / workflow_run — head_branch=main, the check does NOT attach to +# the PR head, so superseding an in-flight run is SAFE. Both events collapse +# onto one group per PR (`…-pr-`) with cancel-in-progress: true. +# • pull_request / pull_request_review — run on the PR head; a cancelled run +# would leave a cancelled `pr-auto-review / check-and-dispatch` check on the +# head. They get a unique-per-run group and never cancel, so they behave +# exactly as before. +# A default-branch trigger with no associated PR (fork / no PR) also falls back to +# the unique-per-run group so unrelated PRs never collapse into one group. +concurrency: + group: >- + ${{ + (github.event_name == 'check_suite' && github.event.check_suite.pull_requests[0].number) + && format('pr-auto-review-ready-check-pr-{0}', github.event.check_suite.pull_requests[0].number) + || (github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number) + && format('pr-auto-review-ready-check-pr-{0}', github.event.workflow_run.pull_requests[0].number) + || format('pr-auto-review-ready-check-unique-{0}', github.run_id) + }} + cancel-in-progress: ${{ github.event_name == 'check_suite' || github.event_name == 'workflow_run' }} + jobs: pr-auto-review: permissions: diff --git a/standards/workflows/pr-auto-review.yml b/standards/workflows/pr-auto-review.yml index 4bc47ddaf..73df2cea4 100644 --- a/standards/workflows/pr-auto-review.yml +++ b/standards/workflows/pr-auto-review.yml @@ -44,6 +44,29 @@ on: permissions: {} +# Deduplicate ONLY the default-branch-context triggers (issue #1126). On GitHub +# Free (20 concurrent jobs, org-wide) this stub fanned out per trigger. Its runs +# split by context: +# • check_suite / workflow_run — head_branch=main, the check does NOT attach to +# the PR head, so superseding an in-flight run is SAFE. Both events collapse +# onto one group per PR (`…-pr-`) with cancel-in-progress: true. +# • pull_request / pull_request_review — run on the PR head; a cancelled run +# would leave a cancelled `pr-auto-review / check-and-dispatch` check on the +# head. They get a unique-per-run group and never cancel, so they behave +# exactly as before. +# A default-branch trigger with no associated PR (fork / no PR) also falls back to +# the unique-per-run group so unrelated PRs never collapse into one group. +concurrency: + group: >- + ${{ + (github.event_name == 'check_suite' && github.event.check_suite.pull_requests[0].number) + && format('pr-auto-review-ready-check-pr-{0}', github.event.check_suite.pull_requests[0].number) + || (github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number) + && format('pr-auto-review-ready-check-pr-{0}', github.event.workflow_run.pull_requests[0].number) + || format('pr-auto-review-ready-check-unique-{0}', github.run_id) + }} + cancel-in-progress: ${{ github.event_name == 'check_suite' || github.event_name == 'workflow_run' }} + jobs: pr-auto-review: permissions: diff --git a/test/workflows/pr-auto-review/concurrency.bats b/test/workflows/pr-auto-review/concurrency.bats new file mode 100644 index 000000000..783364cd8 --- /dev/null +++ b/test/workflows/pr-auto-review/concurrency.bats @@ -0,0 +1,148 @@ +#!/usr/bin/env bats +# Tests for the caller-stub workflow-level `concurrency:` block that deduplicates +# ONLY the default-branch-context Ready Check triggers (issue #1126). +# +# Background: on GitHub Free (20 concurrent jobs, org-wide) the Ready Check stub +# had NO concurrency and fanned out per trigger. Its runs split by trigger: +# • check_suite / workflow_run — head_branch=main, default-branch context, the +# check does NOT attach to the PR head, so superseding these is SAFE. +# • pull_request / pull_request_review — run on the PR head; a cancelled run +# would leave a cancelled `pr-auto-review / check-and-dispatch` check on the +# PR head (the #1422/#1427 stranding shape), so these MUST stay unique. +# +# The block therefore keys check_suite + workflow_run on the PR number (shared, +# cancel-in-progress: true) and gives every PR-context run — and any +# default-branch trigger with no associated PR (fork / no PR) — a unique group. + +load 'helpers/setup' + +STUB="${TT_REPO_ROOT}/standards/workflows/pr-auto-review.yml" +LIVE="${TT_REPO_ROOT}/.github/workflows/pr-auto-review.yml" + +# Exact, pinned expressions. yq folds the `>-` block scalar to one space-joined +# line, so these are the precise resolved strings the stub must carry. Pinning +# them guarantees the live YAML implements exactly the logic the resolver +# functions below describe. +GROUP_EXPR="\${{ (github.event_name == 'check_suite' && github.event.check_suite.pull_requests[0].number) && format('pr-auto-review-ready-check-pr-{0}', github.event.check_suite.pull_requests[0].number) || (github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number) && format('pr-auto-review-ready-check-pr-{0}', github.event.workflow_run.pull_requests[0].number) || format('pr-auto-review-ready-check-unique-{0}', github.run_id) }}" + +CANCEL_EXPR="\${{ github.event_name == 'check_suite' || github.event_name == 'workflow_run' }}" + +# resolve_group / resolve_cancel encode the specified behavior of the pinned +# expressions above. . +resolve_group() { + local event="$1" pr="$2" run_id="$3" + if { [ "$event" = "check_suite" ] || [ "$event" = "workflow_run" ]; } && [ -n "$pr" ]; then + printf 'pr-auto-review-ready-check-pr-%s' "$pr" + else + printf 'pr-auto-review-ready-check-unique-%s' "$run_id" + fi +} + +resolve_cancel() { + local event="$1" + if [ "$event" = "check_suite" ] || [ "$event" = "workflow_run" ]; then + echo "true" + else + echo "false" + fi +} + +# -------------------------------------------------------------------------- +# The stub carries the concurrency block and the two expressions are pinned. +# -------------------------------------------------------------------------- + +@test "stub: defines a workflow-level concurrency block" { + run yq -r '.concurrency | has("group")' "$STUB" + [ "$status" -eq 0 ] + [ "$output" = "true" ] +} + +@test "stub: group expression is exactly the PR-keyed / unique-fallback expression" { + run yq -r '.concurrency.group' "$STUB" + [ "$status" -eq 0 ] + [ "$output" = "$GROUP_EXPR" ] +} + +@test "stub: cancel-in-progress is true only for the two default-branch-context events" { + run yq -r '.concurrency.cancel-in-progress' "$STUB" + [ "$status" -eq 0 ] + [ "$output" = "$CANCEL_EXPR" ] +} + +# -------------------------------------------------------------------------- +# The group expression uses the correct per-event pull_requests[0].number path. +# -------------------------------------------------------------------------- + +@test "stub: check_suite branch reads github.event.check_suite.pull_requests[0].number" { + run yq -r '.concurrency.group' "$STUB" + [[ "$output" == *"github.event.check_suite.pull_requests[0].number"* ]] +} + +@test "stub: workflow_run branch reads github.event.workflow_run.pull_requests[0].number" { + run yq -r '.concurrency.group' "$STUB" + [[ "$output" == *"github.event.workflow_run.pull_requests[0].number"* ]] +} + +@test "stub: fallback group is unique per run (github.run_id)" { + run yq -r '.concurrency.group' "$STUB" + [[ "$output" == *"format('pr-auto-review-ready-check-unique-{0}', github.run_id)"* ]] +} + +@test "stub: PR-context events are NOT keyed on a pull_request number (they hit the unique fallback)" { + run yq -r '.concurrency.group' "$STUB" + # neither pull_request nor pull_request_review appear in the group expression, + # so both fall through to the unique-per-run fallback. + [[ "$output" != *"pull_request.number"* ]] + [[ "$output" != *"event.pull_request"* ]] +} + +# -------------------------------------------------------------------------- +# Resolved group + cancel for each of the four events (spec behavior). +# -------------------------------------------------------------------------- + +@test "resolve: check_suite with PR #1793 dedupes on the PR number and cancels in progress" { + [ "$(resolve_group check_suite 1793 42)" = "pr-auto-review-ready-check-pr-1793" ] + [ "$(resolve_cancel check_suite)" = "true" ] +} + +@test "resolve: workflow_run with PR #1793 shares the SAME group as check_suite and cancels" { + # both default-branch-context events collapse onto one group per PR + [ "$(resolve_group workflow_run 1793 99)" = "pr-auto-review-ready-check-pr-1793" ] + [ "$(resolve_group workflow_run 1793 99)" = "$(resolve_group check_suite 1793 42)" ] + [ "$(resolve_cancel workflow_run)" = "true" ] +} + +@test "resolve: pull_request gets a unique-per-run group and never cancels" { + [ "$(resolve_group pull_request "" 55)" = "pr-auto-review-ready-check-unique-55" ] + [ "$(resolve_cancel pull_request)" = "false" ] +} + +@test "resolve: pull_request_review gets a unique-per-run group and never cancels" { + [ "$(resolve_group pull_request_review "" 77)" = "pr-auto-review-ready-check-unique-77" ] + [ "$(resolve_cancel pull_request_review)" = "false" ] +} + +@test "resolve: check_suite with NO associated PR falls back to a unique-per-run group" { + [ "$(resolve_group check_suite "" 123)" = "pr-auto-review-ready-check-unique-123" ] +} + +@test "resolve: two different PRs never collapse into one group" { + [ "$(resolve_group check_suite 1793 1)" != "$(resolve_group check_suite 1794 1)" ] +} + +# -------------------------------------------------------------------------- +# Template and this repo's live copy carry an identical concurrency block, so +# a standards resync will not revert it and the surface-drift guard stays clean. +# -------------------------------------------------------------------------- + +@test "live copy: group expression matches the template exactly" { + run yq -r '.concurrency.group' "$LIVE" + [ "$status" -eq 0 ] + [ "$output" = "$GROUP_EXPR" ] +} + +@test "live copy: cancel-in-progress matches the template exactly" { + run yq -r '.concurrency.cancel-in-progress' "$LIVE" + [ "$status" -eq 0 ] + [ "$output" = "$CANCEL_EXPR" ] +} From 6417349f6b03cb70795fbbf4159e051065fcd3d3 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:27:43 +0000 Subject: [PATCH 2/2] fix(reviews): address review comments [skip ci-relay] --- test/workflows/pr-auto-review/concurrency.bats | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/workflows/pr-auto-review/concurrency.bats b/test/workflows/pr-auto-review/concurrency.bats index 783364cd8..b3de744da 100644 --- a/test/workflows/pr-auto-review/concurrency.bats +++ b/test/workflows/pr-auto-review/concurrency.bats @@ -75,21 +75,25 @@ resolve_cancel() { @test "stub: check_suite branch reads github.event.check_suite.pull_requests[0].number" { run yq -r '.concurrency.group' "$STUB" + [ "$status" -eq 0 ] [[ "$output" == *"github.event.check_suite.pull_requests[0].number"* ]] } @test "stub: workflow_run branch reads github.event.workflow_run.pull_requests[0].number" { run yq -r '.concurrency.group' "$STUB" + [ "$status" -eq 0 ] [[ "$output" == *"github.event.workflow_run.pull_requests[0].number"* ]] } @test "stub: fallback group is unique per run (github.run_id)" { run yq -r '.concurrency.group' "$STUB" + [ "$status" -eq 0 ] [[ "$output" == *"format('pr-auto-review-ready-check-unique-{0}', github.run_id)"* ]] } @test "stub: PR-context events are NOT keyed on a pull_request number (they hit the unique fallback)" { run yq -r '.concurrency.group' "$STUB" + [ "$status" -eq 0 ] # neither pull_request nor pull_request_review appear in the group expression, # so both fall through to the unique-per-run fallback. [[ "$output" != *"pull_request.number"* ]]