From 9bfb6fce57f839aa746ef87c566bcf4c39cfb02e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 3 Sep 2026 15:04:18 +0900 Subject: [PATCH] fix(ci): dedupe strix.yml's cancel-superseded-pr-runs cleanup job The cleanup job that retires superseded/closed-PR Strix scan runs had no concurrency group of its own, so a push burst spawned N independent instances that each competed 1:1 for the same scarce 60-job Actions-plan admission ceiling the job exists to relieve -- the identical failure class cbd1280 already fixed for current-head-run-coalescer.yml, just not yet applied here. Adds a PR-scoped (repo+PR-number), cancel-in-progress:true concurrency group, matching codeql-pr.yml's established group-key style. cancel-in-progress:true (not queue:max, which the coalescer needs because each of its queued instances carries a different specific expected-head only it can act on) is correct here because every sweep re-verifies live PR state before selecting or cancelling anything, so a fresh instance always fully subsumes whatever an older, not-yet-run instance would have done -- nothing is lost by cancelling a stale queued/running sweep. Originally designed to apply the same fix to opencode-review.yml's sibling cancel-superseded-opencode-review-runs job. Dropped that half after discovering 8141b99 (landed on origin/main mid-session, after the design was written) already restructured that workflow's WORKFLOW-level concurrency group to repo+PR-scoped/cancel-in-progress:false -- a strictly stronger fix that caps the entire run (including this cleanup job) to at most one active instance per PR, for every event type. A job-level dedup on top would have been redundant on the common path, and its planned comment (asserting the outer group "must stay head-scoped") would no longer match the file. Required companion test edit: test_strix_serializes_provider_evidence_per_repository_and_pr isolates the strix: job's own text before locating "concurrency:", since cancel-superseded-pr-runs above it now carries its own block and would otherwise win the naive first-match split. Verified: coverage run -m pytest tests (2680 passed, 1 skipped), coverage report --show-missing (100%), interrogate (100%), actionlint (clean except a pre-existing unrelated shellcheck style note), yaml.safe_load on the touched workflow. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/strix.yml | 16 ++++++++++++++++ tests/test_required_workflow_queue_contract.py | 7 ++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 32a72ad664..c181e2a84d 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -146,6 +146,22 @@ jobs: cancel-superseded-pr-runs: if: github.event_name == 'pull_request_target' && (github.event.action == 'synchronize' || github.event.action == 'closed') + # Idempotent per PR: a fresh sweep re-verifies live state (live_target_matches + # below) before selecting or cancelling anything, so it fully subsumes + # whatever an older, not-yet-run instance would have done. cancel-in-progress + # true is the right shape here (current-head-run-coalescer.yml instead uses + # its own admission-order queueing, since each of its queued instances + # carries a DIFFERENT specific expected-head only it can act on): it caps + # this job to one running + one queued per PR instead of letting a push + # burst pile up N independent, mutually-non-deduped sweeps that each cost a + # full admission slot under the shared 60-job ceiling. Matches + # codeql-pr.yml's established group-key style (PR-number scoped). + concurrency: + group: >- + cancel-superseded-pr-runs-${{ + github.event.pull_request.base.repo.full_name || github.repository }}-${{ + github.event.pull_request.number || github.run_id }} + cancel-in-progress: true runs-on: ubuntu-24.04 # Bound this gh-api-only cleanup job so a stuck call (rate limit, hung # `gh api --paginate`) cannot silently occupy a runner for GitHub's diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index f2a2a9c1d7..82bca9448e 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -365,7 +365,12 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None: scans for *other* PRs to be blocked by it. """ workflow = workflow_text("strix.yml") - concurrency_contract = workflow.split("concurrency:", 1)[1].split( + # Isolate the strix: job's own text first: cancel-superseded-pr-runs above + # it now carries its own (PR-scoped, dedup-only) concurrency: block, so a + # naive first-match split on the bare "concurrency:" literal would grab + # that job's block instead of this one. + strix_job = workflow.split("\n strix:\n", 1)[1] + concurrency_contract = strix_job.split("concurrency:", 1)[1].split( "permissions:", 1 )[0]