From 00fc051a8fa5ca1fafcf5822385eda02d957da87 Mon Sep 17 00:00:00 2001 From: Shai Shmuel Date: Wed, 4 Nov 2026 16:29:38 +0200 Subject: [PATCH 1/3] ci(release): stop unapproved pushes holding the release queue Every Release run shared one queue, and each push to main needs environment approval. One unapproved push therefore blocked later pushes and publication, until the queue overflowed and new runs failed at startup. Give each push run its own group, and let the newest push replace an older authoring job through a separate group. Publication and recovery keep their queue with no cancellation. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 10 +++- RELEASE.md | 71 +++++++++++++++++-------- tools/ci/check_release_contract.py | 24 +++++++++ tools/ci/tests/test_release_contract.py | 40 ++++++++++++++ 4 files changed, 122 insertions(+), 23 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cbf70af1d..58d37d8c7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,7 +25,9 @@ permissions: contents: read concurrency: - group: public-release-main + # Publication and recovery share one queue and are never cancelled. Each push run + # gets its own group, so a job waiting for approval cannot hold or overflow it. + group: ${{ github.event_name == 'push' && format('public-release-push-{0}', github.run_id) || 'public-release-main' }} queue: max cancel-in-progress: false @@ -40,6 +42,12 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 30 environment: release-automation + # The newest push replaces an older authoring job, including one waiting for + # approval. Other events skip this job and must never cancel it: the release + # event that starts publication arrives while this job is still running. + concurrency: + group: public-release-authoring + cancel-in-progress: ${{ github.event_name == 'push' }} permissions: contents: read outputs: diff --git a/RELEASE.md b/RELEASE.md index 9e22d8626..67c7c7153 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -109,13 +109,28 @@ be published with the later run's provenance. The App credential is required because its events trigger workflows; a release created with `GITHUB_TOKEN` does not provide that handoff. Publication does not depend on a tag-push event. -Release work is serialized with GitHub's `queue: max` concurrency mode and no -in-progress cancellation, so newer events do not replace pending releases. The -queue has GitHub's documented bound of 100 pending runs. The current actionlint -schema does not recognize `queue`; CI exempts only that diagnostic and separately -checks the required queue setting. See [workflow +Publication and manual recovery runs are serialized in the `public-release-main` +group with GitHub's `queue: max` concurrency mode and no in-progress cancellation, +so newer events do not replace pending releases. The queue has GitHub's documented +bound of 100 pending runs. The current actionlint schema does not recognize +`queue`; CI exempts only that diagnostic and separately checks the required queue +setting. See [workflow concurrency](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency). +Push runs stay out of that queue, because each one needs `release-automation` +approval and would hold the queue while it waits. A push run has a group of its +own, and its `release-please` job joins the separate `public-release-authoring` +group, where a newer push cancels the older job. GitHub does not allow `queue: max` +with in-progress cancellation, so that group uses the default single queue. No +release is lost when a job is replaced: the next approved job reads the repository +state, finds any merged release PR still awaiting its release, and rebuilds the open +release PR. + +Authoring is therefore not serialized against publication. A release PR merged while +an earlier release is publishing can create the newer release before that run ends. +The newer release's run waits in the queue, and the earlier run leaves the floating +image aliases for it to move. + PR and candidate builds produce archives without publishing. They use the actual package versions in that source tree. Release builds additionally require the complete package set to match the release version. @@ -318,24 +333,36 @@ any automation repair through normal review and CI, then approve that commit's `release-automation` job. The manual entrypoint below only recovers failed publication; it does not refresh release PRs. -Before resuming a waiting authoring run, inspect its source revision and workflow: -release-please can read current `main`, but the run still executes the workflow -from its original push. An old run can therefore generate current release files -using outdated refresh steps. Cancel obsolete queued **push** runs before -cancelling the old waiting run, then retain the authoring run from the intended -current source. Do not cancel publication or recovery runs as part of this -cleanup. Keep the environment protections and approve the retained run normally. -If the intended push run was cancelled, rerun that exact push run after checking -its source and workflow, while GitHub still permits the rerun; do not replay the -obsolete backlog. Verify the generated PR's source ancestry after refresh, too: -the approved run's revision does not freeze release-please's reads of `main`. - -An authoring run left waiting for approval holds the release concurrency group, -so every later push queues behind it. Once 100 runs are pending, the queue is -full and GitHub does not admit further `Release` runs. Each one appears as a +Each push replaces the previous authoring job, so earlier push runs show as +cancelled and normally only the newest push is waiting for approval. Approve that +one. If another push lands while the approved job is running, that job is cancelled +too; approve the newer run. A job cancelled between creating a release and updating +the release PR's label can leave the next run failing once on the duplicate release. +That run corrects the label, and the run after it proceeds normally. + +GitHub's documentation does not say how in-progress cancellation treats a job +waiting on an environment. If an older authoring job ever remains `waiting` while +a newer push's job is pending, cancel the older run rather than approving it. Do +not cancel publication or recovery runs as part of this cleanup. Keep the +environment protections and approve the retained run normally. + +Before approving or rerunning an authoring run, inspect its source revision and +workflow: release-please can read current `main`, but the run still executes the +workflow from its original push. An old run can therefore generate current release +files using outdated refresh steps. If the newest push run was cancelled, rerun +that exact push run after checking its source and workflow, while GitHub still +permits the rerun; do not replay older runs. Verify the generated PR's source +ancestry after refresh, too: the approved run's revision does not freeze +release-please's reads of `main`. + +An unapproved push run does not delay publication. A publication or recovery run +waiting on an environment approval does hold `public-release-main`, including a +dispatched recovery run nobody has approved, so resolve those first if publication +appears stuck. If 100 runs were ever pending there, the queue would be full and +GitHub would not admit further publication or recovery runs. Each one appears as a failed run with zero jobs, and GitHub reports that it likely failed because of a -workflow file issue. Before debugging the workflow file, look for a `Release` -run in the `waiting` state and apply the cleanup above. +workflow file issue. Before debugging the workflow file, look for a `Release` run +in the `waiting` state. ### Artifact publication diff --git a/tools/ci/check_release_contract.py b/tools/ci/check_release_contract.py index d4f0ca645..5dba9f601 100755 --- a/tools/ci/check_release_contract.py +++ b/tools/ci/check_release_contract.py @@ -527,9 +527,21 @@ def license_policy_errors() -> list[str]: ) +RELEASE_QUEUE_GROUP = ( + "${{ github.event_name == 'push' && format('public-release-push-{0}', github.run_id) || 'public-release-main' }}" +) +RELEASE_AUTHORING_CONCURRENCY = ( + " concurrency:\n" + " group: public-release-authoring\n" + " cancel-in-progress: ${{ github.event_name == 'push' }}\n" +) + + def release_queue_errors(top: str, ci: str) -> list[str]: errors = [] concurrency = top.partition("concurrency:\n")[2].partition("\njobs:")[0] + if re.findall(r"^ group: (.+)$", concurrency, re.MULTILINE) != [RELEASE_QUEUE_GROUP]: + errors.append("each push run needs its own group so an approval wait cannot hold the publication queue") if re.findall(r"^ queue: (.+)$", concurrency, re.MULTILINE) != ["max"]: errors.append("release concurrency must preserve all pending runs with queue: max") if re.findall(r"^ cancel-in-progress: (.+)$", concurrency, re.MULTILINE) != ["false"]: @@ -539,6 +551,17 @@ def release_queue_errors(top: str, ci: str) -> list[str]: return errors +def release_authoring_concurrency_errors(blocks: dict[str, str]) -> list[str]: + errors = [] + authoring = blocks.get("release-please", "") + if authoring.count("concurrency:") != 1 or RELEASE_AUTHORING_CONCURRENCY not in authoring: + errors.append("a newer push must replace an older release-please job through its own concurrency group") + others = [name for name, block in blocks.items() if name != "release-please" and "concurrency:" in block] + if others: + errors.append(f"job-level concurrency could cancel or displace release work: {others}") + return errors + + def artifact_retention_errors(name: str, text: str) -> list[str]: errors = [] for retention in re.findall(r"^\s*retention-days:\s*([^\n]*)$", text, re.MULTILINE): @@ -623,6 +646,7 @@ def release_workflow_errors() -> list[str]: ): errors.append("release candidate rehearsal must be read-only and unable to publish") errors.extend(release_queue_errors(top, (ROOT / ".github/workflows/ci.yml").read_text())) + errors.extend(release_authoring_concurrency_errors(blocks)) recover = blocks.get("recover", "") if "release_recovery" not in recover or "id-token:" in recover or "publish-" in recover: errors.append("manual recovery must only rerun the original jobs") diff --git a/tools/ci/tests/test_release_contract.py b/tools/ci/tests/test_release_contract.py index 72b21008c..563f5d67e 100644 --- a/tools/ci/tests/test_release_contract.py +++ b/tools/ci/tests/test_release_contract.py @@ -461,6 +461,46 @@ def test_release_queue_cancellation_and_linter_exception_are_exact(): assert not re.fullmatch(contract.QUEUE_SCHEMA_DIAGNOSTIC, diagnostic.replace('"queue"', '"bogus"')) +@pytest.mark.parametrize( + "group", + [ + "public-release-main", + "${{ github.event_name == 'push' && 'public-release-push' || 'public-release-main' }}", + "${{ github.event_name == 'release' && format('run-{0}', github.run_id) || 'public-release-main' }}", + ], +) +def test_push_runs_cannot_hold_the_publication_queue(group): + top = (contract.ROOT / ".github/workflows/release.yml").read_text() + ci = (contract.ROOT / ".github/workflows/ci.yml").read_text() + assert contract.RELEASE_QUEUE_GROUP in top + assert contract.release_queue_errors(top.replace(contract.RELEASE_QUEUE_GROUP, group), ci) + + +@pytest.mark.parametrize( + "replacement", + [ + "", + " concurrency:\n group: public-release-main\n cancel-in-progress: ${{ github.event_name == 'push' }}\n", + " concurrency:\n group: public-release-authoring\n cancel-in-progress: false\n", + " concurrency:\n group: public-release-authoring\n cancel-in-progress: true\n", + " concurrency:\n group: public-release-authoring\n queue: max\n", + ], +) +def test_newest_push_must_replace_the_authoring_job(replacement): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + assert contract.release_authoring_concurrency_errors(blocks) == [] + assert contract.RELEASE_AUTHORING_CONCURRENCY in blocks["release-please"] + authoring = blocks["release-please"].replace(contract.RELEASE_AUTHORING_CONCURRENCY, replacement) + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": authoring}) + + +@pytest.mark.parametrize("job", ["prepare", "python-publish", "docker", "complete", "recover"]) +def test_only_authoring_may_use_job_level_concurrency(job): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + changed = blocks[job] + "\n concurrency:\n group: public-release-authoring\n" + assert contract.release_authoring_concurrency_errors({**blocks, job: changed}) + + def test_authoring_no_longer_assumes_release_sha_is_push_sha(): jobs = contract.workflow_job_blocks(".github/workflows/release.yml") assert 'test "$RELEASE_SHA" = "$EXPECTED_SHA"' not in jobs["release-please"] From dd45363ee593723617db887fb8bf4c522ca1706f Mon Sep 17 00:00:00 2001 From: Shai Shmuel Date: Wed, 4 Nov 2026 17:19:32 +0200 Subject: [PATCH 2/3] test(release): keep a contract test case within the line limit CI lints tools/ci with an explicit rule selection that includes E501. Derive the case from the pinned constant, which also keeps the two from drifting. Co-Authored-By: Claude Opus 5.5 --- tools/ci/tests/test_release_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci/tests/test_release_contract.py b/tools/ci/tests/test_release_contract.py index 563f5d67e..3d3ff8732 100644 --- a/tools/ci/tests/test_release_contract.py +++ b/tools/ci/tests/test_release_contract.py @@ -480,7 +480,7 @@ def test_push_runs_cannot_hold_the_publication_queue(group): "replacement", [ "", - " concurrency:\n group: public-release-main\n cancel-in-progress: ${{ github.event_name == 'push' }}\n", + contract.RELEASE_AUTHORING_CONCURRENCY.replace("public-release-authoring", "public-release-main"), " concurrency:\n group: public-release-authoring\n cancel-in-progress: false\n", " concurrency:\n group: public-release-authoring\n cancel-in-progress: true\n", " concurrency:\n group: public-release-authoring\n queue: max\n", From 49ca5e65016849bd706d68bd18f65e29fea04073 Mon Sep 17 00:00:00 2001 From: Shai Shmuel Date: Wed, 4 Nov 2026 17:32:29 +0200 Subject: [PATCH 3/3] ci(release): refuse an authoring run behind main GitHub does not guarantee that jobs enter a concurrency group in push order, so an older job can replace a newer one and run outdated refresh steps against the current release PR. Fail before any credentials are used unless the run's commit is the newest on main. Also compare the complete authoring concurrency mapping, so an appended key such as queue: max no longer passes the contract check. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 9 +++++ RELEASE.md | 29 ++++++++-------- tools/ci/check_release_contract.py | 16 ++++++++- tools/ci/tests/test_release_contract.py | 45 +++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 15 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 58d37d8c7..ecf0d91a1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -53,6 +53,15 @@ jobs: outputs: release_pr_head: ${{ steps.release-pr-head.outputs.sha }} steps: + - name: Refuse a run whose push is no longer the newest on main + run: | + set -euo pipefail + main_sha="$(git ls-remote "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY.git" refs/heads/main | cut -f1)" + [[ "$main_sha" =~ ^[0-9a-f]{40}$ ]] + if [ "$main_sha" != "$GITHUB_SHA" ]; then + echo "::error::$GITHUB_SHA is behind main ($main_sha); approve or rerun the newest push run." + exit 1 + fi - name: Require activated public release GitHub App credentials env: APP_ID: ${{ vars.PUBLIC_RELEASE_APP_ID }} diff --git a/RELEASE.md b/RELEASE.md index 67c7c7153..72281d1c1 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -340,20 +340,21 @@ too; approve the newer run. A job cancelled between creating a release and updat the release PR's label can leave the next run failing once on the duplicate release. That run corrects the label, and the run after it proceeds normally. -GitHub's documentation does not say how in-progress cancellation treats a job -waiting on an environment. If an older authoring job ever remains `waiting` while -a newer push's job is pending, cancel the older run rather than approving it. Do -not cancel publication or recovery runs as part of this cleanup. Keep the -environment protections and approve the retained run normally. - -Before approving or rerunning an authoring run, inspect its source revision and -workflow: release-please can read current `main`, but the run still executes the -workflow from its original push. An old run can therefore generate current release -files using outdated refresh steps. If the newest push run was cancelled, rerun -that exact push run after checking its source and workflow, while GitHub still -permits the rerun; do not replay older runs. Verify the generated PR's source -ancestry after refresh, too: the approved run's revision does not freeze -release-please's reads of `main`. +release-please reads current `main`, but a run executes the workflow from its +original push, so an old run could generate current release files using outdated +refresh steps. GitHub does not guarantee that jobs enter the group in push order, +and its documentation does not say how in-progress cancellation treats a job +waiting on an environment. An older job can therefore occasionally replace a newer +one or stay `waiting` ahead of it. To keep that harmless, the job's first step fails +before it uses any credentials unless its commit is the newest on `main`. + +If an older job is waiting, cancel it rather than approving it. If the newest push +run was cancelled, rerun that exact push run while GitHub still permits the rerun; +do not replay older runs. If `main` moved without starting a run, the next push +starts a current one. Do not cancel publication or recovery runs as part of this +cleanup. Keep the environment protections and approve the retained run normally. +Verify the generated PR's source ancestry after refresh, too: the approved run's +revision does not freeze release-please's reads of `main`. An unapproved push run does not delay publication. A publication or recovery run waiting on an environment approval does hold `public-release-main`, including a diff --git a/tools/ci/check_release_contract.py b/tools/ci/check_release_contract.py index 5dba9f601..e733a943e 100755 --- a/tools/ci/check_release_contract.py +++ b/tools/ci/check_release_contract.py @@ -535,6 +535,17 @@ def license_policy_errors() -> list[str]: " group: public-release-authoring\n" " cancel-in-progress: ${{ github.event_name == 'push' }}\n" ) +RELEASE_AUTHORING_FRESHNESS_STEP = """\ + - name: Refuse a run whose push is no longer the newest on main + run: | + set -euo pipefail + main_sha="$(git ls-remote "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY.git" refs/heads/main | cut -f1)" + [[ "$main_sha" =~ ^[0-9a-f]{40}$ ]] + if [ "$main_sha" != "$GITHUB_SHA" ]; then + echo "::error::$GITHUB_SHA is behind main ($main_sha); approve or rerun the newest push run." + exit 1 + fi +""" def release_queue_errors(top: str, ci: str) -> list[str]: @@ -554,8 +565,11 @@ def release_queue_errors(top: str, ci: str) -> list[str]: def release_authoring_concurrency_errors(blocks: dict[str, str]) -> list[str]: errors = [] authoring = blocks.get("release-please", "") - if authoring.count("concurrency:") != 1 or RELEASE_AUTHORING_CONCURRENCY not in authoring: + mappings = re.findall(r"^ concurrency:\n(?: .*(?:\n|\Z))*", authoring, re.MULTILINE) + if mappings != [RELEASE_AUTHORING_CONCURRENCY]: errors.append("a newer push must replace an older release-please job through its own concurrency group") + if not authoring.partition(" steps:\n")[2].startswith(RELEASE_AUTHORING_FRESHNESS_STEP): + errors.append("release-please must refuse a push that is behind main before using any credentials") others = [name for name, block in blocks.items() if name != "release-please" and "concurrency:" in block] if others: errors.append(f"job-level concurrency could cancel or displace release work: {others}") diff --git a/tools/ci/tests/test_release_contract.py b/tools/ci/tests/test_release_contract.py index 3d3ff8732..db8d7a70c 100644 --- a/tools/ci/tests/test_release_contract.py +++ b/tools/ci/tests/test_release_contract.py @@ -484,6 +484,7 @@ def test_push_runs_cannot_hold_the_publication_queue(group): " concurrency:\n group: public-release-authoring\n cancel-in-progress: false\n", " concurrency:\n group: public-release-authoring\n cancel-in-progress: true\n", " concurrency:\n group: public-release-authoring\n queue: max\n", + contract.RELEASE_AUTHORING_CONCURRENCY + " queue: max\n", ], ) def test_newest_push_must_replace_the_authoring_job(replacement): @@ -501,6 +502,50 @@ def test_only_authoring_may_use_job_level_concurrency(job): assert contract.release_authoring_concurrency_errors({**blocks, job: changed}) +def test_authoring_must_check_freshness_before_any_other_step(): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + authoring = blocks["release-please"] + step = contract.RELEASE_AUTHORING_FRESHNESS_STEP + assert step in authoring + removed = authoring.replace(step, "") + moved = removed + "\n" + step + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": removed}) + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": moved}) + + +def test_authoring_freshness_step_refuses_a_push_behind_main(tmp_path): + work = tmp_path / "work" + remote = tmp_path / "owner" / "repo.git" + git_env = {**os.environ, "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_NOSYSTEM": "1"} + + def git(*args, cwd=work): + command = ["git", *args] + result = subprocess.run(command, cwd=cwd, env=git_env, check=True, capture_output=True, text=True) # noqa: S603 + return result.stdout.strip() + + work.mkdir() + git("init", "-q", "-b", "main") + identity = ("-c", "user.name=test", "-c", "user.email=test@example.invalid") + git(*identity, "commit", "-q", "--allow-empty", "-m", "older") + older = git("rev-parse", "HEAD") + git(*identity, "commit", "-q", "--allow-empty", "-m", "newer") + newer = git("rev-parse", "HEAD") + git("clone", "-q", "--bare", str(work), str(remote)) + script = textwrap.dedent(contract.RELEASE_AUTHORING_FRESHNESS_STEP.partition("run: |\n")[2]) + + def run_step(sha): + env = {**git_env, "GITHUB_SERVER_URL": tmp_path.as_uri(), "GITHUB_REPOSITORY": "owner/repo", "GITHUB_SHA": sha} + command = ["bash", "-c", script] + return subprocess.run(command, cwd=tmp_path, env=env, check=False, capture_output=True, text=True) # noqa: S603 + + assert run_step(newer).returncode == 0 + stale = run_step(older) + assert stale.returncode == 1 + assert f"::error::{older} is behind main ({newer})" in stale.stdout + git("update-ref", "-d", "refs/heads/main", cwd=remote) + assert run_step(newer).returncode != 0 + + def test_authoring_no_longer_assumes_release_sha_is_push_sha(): jobs = contract.workflow_job_blocks(".github/workflows/release.yml") assert 'test "$RELEASE_SHA" = "$EXPECTED_SHA"' not in jobs["release-please"]