Skip to content

feat: implement issue #1126 — Free-plan 20-job cap: PR Auto-Review Ready Check has no concurrency and fans out per check_suite/workflow_run — dedupe the default-branch-context runs only #1131

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
don-petry merged 3 commits into from
Sep 16, 2026
Merged

feat: implement issue #1126 — Free-plan 20-job cap: PR Auto-Review Ready Check has no concurrency and fans out per check_suite/workflow_run — dedupe the default-branch-context runs only #1131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fc1260
feat: implement issue #1126 — Free-plan 20-job cap: PR Auto-Review Re…
don-petry Sep 13, 2026
6417349
fix(reviews): address review comments [skip ci-relay]
don-petry Sep 13, 2026
05fb262
Merge branch 'main' into dev-lead/issue-1126-20260913-0915
don-petry Sep 16, 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
23 changes: 23 additions & 0 deletions .github/workflows/pr-auto-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-<number>`) 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:
Expand Down
23 changes: 23 additions & 0 deletions standards/workflows/pr-auto-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-<number>`) 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:
Expand Down
152 changes: 152 additions & 0 deletions test/workflows/pr-auto-review/concurrency.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
#!/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. <event> <pr-number-or-empty> <run-id>.
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"
[ "$status" -eq 0 ]
[[ "$output" == *"github.event.check_suite.pull_requests[0].number"* ]]
}
Comment thread
don-petry marked this conversation as resolved.

@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"* ]]
[[ "$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" ]
}
Comment thread
don-petry marked this conversation as resolved.
Loading