Skip to content

Queue pull request CI runs per author and workflow #1426

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

Open
dhruv8sh wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Queue pull request CI runs per author and workflow #1426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf76307
Queue pull request CI runs per author and workflow
dhruv8sh Oct 20, 2026
68e005b
Key the CI queue on the PR author and add job timeouts
dhruv8sh Oct 20, 2026
3d448f9
Merge branch 'main' into chore/ci-per-user-concurrency
aram356 Oct 20, 2026
a752e1d
Merge branch 'main' into chore/ci-per-user-concurrency
aram356 Oct 22, 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
11 changes: 11 additions & 0 deletions .github/workflows/codeql.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ on:
schedule:
- cron: "42 6 * * 0"

# Each pull request author runs one PR at a time per workflow, so one person
# opening many PRs cannot fill the runner queue ahead of everyone else. Their
# other runs wait in order instead of being cancelled, and every job has a
# timeout so a hung run cannot hold that queue for hours. Bot-authored PRs and
# other events get their own group from the run ID and are never held back.
concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}
cancel-in-progress: false
queue: max

jobs:
analyze:
name: Analyze (${{ matrix.language }})
Expand All @@ -28,6 +38,7 @@ jobs:
# - https://gh.io/using-larger-runners (GitHub.com only)
# Consider using larger runners or machines with greater resources for possible analysis time improvements.
runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
timeout-minutes: 45
permissions:
# required for all workflows
security-events: write
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/format.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,21 @@ on:
permissions:
contents: read

# Each pull request author runs one PR at a time per workflow, so one person
# opening many PRs cannot fill the runner queue ahead of everyone else. Their
# other runs wait in order instead of being cancelled, and every job has a
# timeout so a hung run cannot hold that queue for hours. Bot-authored PRs and
# other events get their own group from the run ID and are never held back.
concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}
cancel-in-progress: false
queue: max

jobs:
check-claude-md-symlink:
name: CLAUDE.md symlink guard
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v4

Expand Down Expand Up @@ -42,6 +53,7 @@ jobs:
format-rust:
name: cargo fmt
runs-on: ubuntu-latest
timeout-minutes: 25
steps:
- uses: actions/checkout@v4

@aram356 aram356 Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — Nothing enforces the shared concurrency block or the job timeouts, and open PRs already break both.

Trial merges of the open workflow-touching PRs with this head:

Following #1259's precedent of a guard step in the required cargo fmt job, this step fails when any PR-triggered workflow's concurrency differs from test.yml's, or when a job lacks timeout-minutes. Jobs that call a reusable workflow are skipped, since they can't set it. It uses the runner image's yq (4.54.1 on ubuntu-latest), so it needs no setup.

Verified on 15 fixtures, locally and under bash 5.2.15 with yq 4.54.1 in Docker. It passes on this head. It fails on trial merges with #1254, #1257, #1433, #1424 and #1345, on main, on one edited group, on every group removed, on a dropped queue, on on: pull_request and on: [push, pull_request] without the block, and on a matrix job missing its timeout. It passes for push-only workflows and for reusable-workflow callers. actionlint and shellcheck report nothing new. If you adopt the job-level design from the other comment, the check moves to the workflow block plus each job's queue.

Suggested change
- uses: actions/checkout@v4
- uses: actions/checkout@v4
# A hung job, or a PR-triggered workflow outside the per-author queue,
# brings back the runner pile-up that the shared concurrency block in
# test.yml prevents. Every PR-triggered workflow must carry that exact
# block, and every job must set timeout-minutes.
- name: Require the per-author CI queue and job timeouts
shell: bash
run: |
expected=$(yq -o=json -I=0 '.concurrency' .github/workflows/test.yml)
case "$expected" in
*pull_request.user.login*) ;;
*) echo "::error file=.github/workflows/test.yml::test.yml must define the per-author concurrency group."; exit 1 ;;
esac
status=0
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] || continue
on_pr=$(yq '.on | ((tag == "!!map" and has("pull_request")) or (tag == "!!str" and . == "pull_request") or (tag == "!!seq" and ([.[] | select(. == "pull_request")] | length > 0)))' "$f")
[ "$on_pr" = true ] || continue
if [ "$(yq -o=json -I=0 '.concurrency' "$f")" != "$expected" ]; then
echo "::error file=$f::PR-triggered workflows must use the concurrency block from test.yml."
status=1
fi
for job in $(yq '.jobs | to_entries[] | select(.value.uses == null and .value["timeout-minutes"] == null) | .key' "$f"); do
echo "::error file=$f::Job '$job' must set timeout-minutes."
status=1
done
done
exit "$status"


Expand Down Expand Up @@ -103,6 +115,7 @@ jobs:

format-typescript:
runs-on: ubuntu-latest
timeout-minutes: 10
defaults:
run:
working-directory: crates/trusted-server-js/lib
Expand Down Expand Up @@ -134,6 +147,7 @@ jobs:

format-docs:
runs-on: ubuntu-latest
timeout-minutes: 10
defaults:
run:
working-directory: docs
Expand Down Expand Up @@ -175,6 +189,7 @@ jobs:
docs-proposal-scripts:
name: docs-proposal scripts
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v4

Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ on:
types: [opened, synchronize, reopened]
workflow_dispatch:

# Each pull request author runs one PR at a time per workflow, so one person
# opening many PRs cannot fill the runner queue ahead of everyone else. Their
# other runs wait in order instead of being cancelled, and every job has a
# timeout so a hung run cannot hold that queue for hours. Bot-authored PRs and
# other events get their own group from the run ID and are never held back.
concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}
cancel-in-progress: false
queue: max

env:
ORIGIN_PORT: 8888
ARTIFACTS_DIR: /tmp/integration-test-artifacts
Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/pbs-example.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ on:
permissions:
contents: read

# Each pull request author runs one PR at a time per workflow, so one person
# opening many PRs cannot fill the runner queue ahead of everyone else. Their
# other runs wait in order instead of being cancelled, and every job has a
# timeout so a hung run cannot hold that queue for hours. Bot-authored PRs and
# other events get their own group from the run ID and are never held back.
concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}
cancel-in-progress: false
queue: max

jobs:
local-checks:
name: PBS Terraform and Compose checks
Expand Down
16 changes: 16 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ on:
branches: [main]
pull_request:

# Each pull request author runs one PR at a time per workflow, so one person
# opening many PRs cannot fill the runner queue ahead of everyone else. Their
# other runs wait in order instead of being cancelled, and every job has a
# timeout so a hung run cannot hold that queue for hours. Bot-authored PRs and

@aram356 aram356 Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 thinking — Bot-authored PRs get a run-ID group, so a Dependabot batch runs fully in parallel. Dependabot opens PRs here in bursts (#1021 to #1024 within 30 seconds, and three ecosystems update weekly), and no one is waiting on those runs, so they are the cheapest ones to queue. Keying bots on their login like everyone else (one dependabot[bot] queue) fits the goal of this PR. #1425 asks for the exemption explicitly, so it would help to say why somewhere. Low stakes this week: 8 of 391 Run Tests PR runs were Dependabot's.

# other events get their own group from the run ID and are never held back.
concurrency:
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}

@aram356 aram356 Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — A per-author queue that never cancels superseded runs backs the required checks up for hours, and it can reach the 100-pending cap.

I replayed the 391 Run Tests PR runs from Oct 18 to 23 under this group. Each run's duration is its uncontended critical path (the longest job), so the replay measures only the delay this policy adds and ignores the runner-pool limit:

  • The run for an author's latest push waits p50 594 min / max 1850 min on jwrosewell's stacked PRs, where a restack fires 18 to 29 runs within two minutes, and p50 104 / max 550 min for prk-Jr, whose release-candidate jobs take one to two hours.
  • 124 of the 391 runs would start after a newer push to the same PR already existed, so most of the queue is stale work.
  • The cap is reachable. The replay peaks at 99 pending, and 5 Run Tests runs (79 CodeQL Advanced runs) for an author's latest push get cancelled, because "When the queue is full, any additional jobs or workflow runs are canceled." Those PRs get no cargo test result for their head without a manual re-run, and the re-run joins the back of a full queue.
  • main requires branches to be up to date, so every merge adds "Update branch" runs to these queues.

The description says cancelling superseded runs needs a separate privileged workflow because a workflow has only one concurrency group. Jobs have their own jobs.<job_id>.concurrency, which accepts queue: max and can read the matrix and strategy contexts. So you can cancel per PR at the workflow level and queue per author at the job level. In the same replay (approximated per run), runner time drops from 138 h to 93 h, no runs are cancelled at the cap, the latest-push wait falls to p50 284 / max 852 min for jwrosewell and p50 0 / max 58 min for prk-Jr, and peak concurrent jobs stay at 56, the same as this PR.

Proposed shape (apply manually: it touches every job in the five workflows):

# One run per PR: a newer push cancels the superseded run.
concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
  cancel-in-progress: true

jobs:
  test-rust:
    # One test-rust job per author at a time; the rest wait in order.
    concurrency:
      group: ${{ github.workflow }}-test-rust-${{ github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot' && github.event.pull_request.user.login || github.run_id }}
      queue: max

Matrix jobs (test-cli, CodeQL analyze) add ${{ matrix.os }} or ${{ matrix.language }} to the job group so their legs don't queue behind each other. Please confirm in your throwaway repo that cancelling a superseded run also drops its jobs still waiting in a job-level queue; I couldn't check that live. If you keep the workflow-level design, the shared comment at lines 11-15 should say that runs past 100 pending are cancelled, since that limit is reachable.

cancel-in-progress: false
queue: max

jobs:
test-rust:
name: cargo test
Expand Down Expand Up @@ -68,6 +78,7 @@ jobs:
test-axum:
name: cargo test (axum native)
runs-on: ubuntu-latest
timeout-minutes: 30

@aram356 aram356 Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

📝 note — Heads-up for the module stack: ten of jwrosewell's PRs (#1347 to #1399, and #1423) get a new test.yml conflict from this PR, because they turn test-axum and test-cloudflare into ubuntu/windows matrices. Once that's resolved, this 30-minute limit applies to the windows-latest axum leg, which ran p50 19.8, p95 21.5 and max 84.2 min across 188 runs. The 3x rule used here would put it near 65. A matrix-aware value keeps both legs covered:

    timeout-minutes: ${{ matrix.os == 'windows-latest' && 65 || 30 }}

The windows cloudflare leg (max 10.0 min) fits in 15.

steps:
- uses: actions/checkout@v4

Expand Down Expand Up @@ -116,6 +127,7 @@ jobs:
test-cloudflare:
name: cargo check (cloudflare native + wasm32-unknown-unknown)
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- uses: actions/checkout@v4

Expand All @@ -137,6 +149,7 @@ jobs:
test-spin:
name: cargo check/build/test (spin native + wasm32-wasip1)
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/checkout@v4

Expand Down Expand Up @@ -170,6 +183,7 @@ jobs:
test-parity:
name: cargo test (cross-adapter parity)
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- uses: actions/checkout@v4

Expand All @@ -195,6 +209,7 @@ jobs:
matrix:
os: [ubuntu-latest, macos-latest]
runs-on: ${{ matrix.os }}
timeout-minutes: 45
steps:
- uses: actions/checkout@v4

Expand Down Expand Up @@ -259,6 +274,7 @@ jobs:
test-typescript:
name: vitest
runs-on: ubuntu-latest
timeout-minutes: 10
defaults:
run:
working-directory: crates/trusted-server-js/lib
Expand Down
Loading