From 63a2851838264460bb59baaa9d11d3c5c3e130a5 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:27:07 +0400 Subject: [PATCH 1/4] Test that only main CI runs can occupy the production deploy queue Require the deploy workflow to filter its workflow_run trigger to main, to declare the calibration-diagnostics-production concurrency group on the deploy job instead of the workflow, and to keep requiring a successful push run on main in every job that holds that group. These fail against the current workflow: the trigger has no branch filter, the group is declared at workflow level, and no job declares it. Refs #260 Co-Authored-By: Claude Opus 5.5 --- frontend/tests/test_deploy_workflow.py | 48 +++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/frontend/tests/test_deploy_workflow.py b/frontend/tests/test_deploy_workflow.py index 2ec913a0..2d0072b2 100644 --- a/frontend/tests/test_deploy_workflow.py +++ b/frontend/tests/test_deploy_workflow.py @@ -52,10 +52,29 @@ def _installs_locked_frontend_dependencies(step, job): ) +PRODUCTION_CONCURRENCY_GROUP = "calibration-diagnostics-production" +SUCCESSFUL_MAIN_PUSH_CLAUSES = { + "github.event.workflow_run.conclusion == 'success'", + "github.event.workflow_run.event == 'push'", + "github.event.workflow_run.head_branch == 'main'", +} + + +def _concurrency_group(job): + concurrency = job.get("concurrency", {}) + return concurrency if isinstance(concurrency, str) else concurrency.get("group") + + def test_deployment_waits_for_successful_main_ci(): + # Without the branch filter, every CI run, pull-request runs included, + # creates a run of this workflow (see #260). workflow = _workflow() assert workflow["on"] == { - "workflow_run": {"workflows": ["CI"], "types": ["completed"]} + "workflow_run": { + "workflows": ["CI"], + "types": ["completed"], + "branches": ["main"], + } } condition = workflow["jobs"]["deploy"]["if"] assert "conclusion == 'success'" in condition @@ -63,6 +82,33 @@ def test_deployment_waits_for_successful_main_ci(): assert "head_branch == 'main'" in condition +def test_production_concurrency_group_belongs_to_the_deploy_job_not_the_workflow(): + # A workflow-level group admits a run as soon as GitHub creates it, before + # its job's `if:` skips it, and replaces any older pending run in the + # group. On 2026-10-21, a pull-request CI run's deploy run, which would + # have skipped, cancelled the pending deploy of #248 that way. + workflow = _workflow() + assert "concurrency" not in workflow + assert workflow["jobs"]["deploy"]["concurrency"] == { + "group": PRODUCTION_CONCURRENCY_GROUP, + "cancel-in-progress": "false", + } + + +def test_every_job_in_the_production_group_requires_a_successful_main_push(): + # The branch filter matches the triggering run's head branch name, and a + # pull request from a fork whose branch is named main passes it. The job + # condition, which requires a push run, is what rejects such a run, so each + # job holding the production group keeps exactly these clauses, all required. + jobs = _workflow()["jobs"] + clauses_by_job = { + job_name: {clause.strip() for clause in job.get("if", "").split("&&")} + for job_name, job in jobs.items() + if _concurrency_group(job) == PRODUCTION_CONCURRENCY_GROUP + } + assert clauses_by_job == {"deploy": SUCCESSFUL_MAIN_PUSH_CLAUSES} + + def test_deployment_uses_exact_project_and_backend_first_order(): workflow = _workflow() assert workflow["jobs"]["deploy"]["environment"] == "Production" From 41071450f0ab31c1433f6acae070113ccd526fe3 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:27:53 +0400 Subject: [PATCH 2/4] Start production deploy runs only for main CI runs The deploy workflow ran after every CI run, pull requests included, and declared its concurrency group at workflow level. Each new run joined the group as soon as GitHub created it, before the deploy job's condition skipped it, and replaced the pending run. On 2026-10-21 a pull-request CI run's deploy run cancelled the pending deploy of #248. Filter the workflow_run trigger to main and declare the concurrency group on the deploy job, so a run whose job is skipped does not take the pending position. The job keeps its condition requiring a successful push run on main, which rejects a pull request from a fork whose branch is named main; that run still passes the branch filter. Fixes #260 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy.yml | 17 +++++++++++++---- frontend/HOSTED_RUNTIME.md | 9 +++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index d7aeac92..1971fc14 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,13 +1,14 @@ name: Deploy production +# Only CI runs whose head branch is main create a run of this workflow. The +# filter matches the branch name alone, so a pull request from a fork whose +# branch is named main still passes it; the deploy job's condition rejects that +# run because its event is pull_request. See #260. on: workflow_run: workflows: [CI] types: [completed] - -concurrency: - group: calibration-diagnostics-production - cancel-in-progress: false + branches: [main] permissions: contents: read @@ -25,6 +26,14 @@ jobs: github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push' && github.event.workflow_run.head_branch == 'main' + # One deploy runs and at most one waits; a newer deploy replaces the waiting + # one, which is safe because the newer main commit contains the older one. + # The group belongs to this job, not the workflow: a workflow-level group + # admits a run when GitHub creates it, before this condition skips it, so a + # run that deploys nothing could replace a waiting deploy of main. + concurrency: + group: calibration-diagnostics-production + cancel-in-progress: false runs-on: ubuntu-latest timeout-minutes: 45 environment: Production diff --git a/frontend/HOSTED_RUNTIME.md b/frontend/HOSTED_RUNTIME.md index 630e6dbd..d177d505 100644 --- a/frontend/HOSTED_RUNTIME.md +++ b/frontend/HOSTED_RUNTIME.md @@ -62,8 +62,13 @@ To upgrade: ## Automated production deployment `.github/workflows/deploy.yml` runs after the `CI` workflow succeeds for a push to -`main`. It checks out the exact commit tested by CI and performs these operations in -order: +`main`. Its trigger accepts only CI runs whose head branch is `main`, and its deploy +job runs only when that CI run was a successful push. The deploy job, not the +workflow, holds the `calibration-diagnostics-production` concurrency group: one deploy +runs, at most one waits, and a newer `main` deploy replaces a waiting one. A run whose +job is skipped, such as one for a pull request from a fork whose branch is named +`main`, must not replace a waiting deploy (#260). The deploy job checks out the exact +commit tested by CI and performs these operations in order: 1. Deploy a commit-specific Modal application in the `main` Modal environment. 2. Resolve the deployed `web_app` URL and verify its authenticated metadata. From 0066f9ef4ee36e7cf471f1fce872c576829d753f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:52:23 +0400 Subject: [PATCH 3/4] Test that the deploy condition check accepts harmless rewordings The check that each job in the production concurrency group requires a successful main push compared the job's `if:` with exactly three `&&`-joined clauses. It rejected conditions that are just as strict: the `${{ }}` expression wrapper, parenthesised or reordered clauses, and an extra required clause such as a repository check. Move the comparison into a helper and test it directly. Three cases fail with the exact comparison: the expression wrapper, reordered and grouped clauses, and an extra required clause. The cases for a missing clause, an `||` alternative and a negation already pass and must keep passing. Co-Authored-By: Claude Opus 5.5 --- frontend/tests/test_deploy_workflow.py | 71 ++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/frontend/tests/test_deploy_workflow.py b/frontend/tests/test_deploy_workflow.py index 2d0072b2..c1a10a39 100644 --- a/frontend/tests/test_deploy_workflow.py +++ b/frontend/tests/test_deploy_workflow.py @@ -95,18 +95,79 @@ def test_production_concurrency_group_belongs_to_the_deploy_job_not_the_workflow } +def _requires_every_clause(condition, required_clauses): + return {clause.strip() for clause in condition.split("&&")} == required_clauses + + def test_every_job_in_the_production_group_requires_a_successful_main_push(): # The branch filter matches the triggering run's head branch name, and a # pull request from a fork whose branch is named main passes it. The job # condition, which requires a push run, is what rejects such a run, so each - # job holding the production group keeps exactly these clauses, all required. + # job holding the production group requires all of these clauses. jobs = _workflow()["jobs"] - clauses_by_job = { - job_name: {clause.strip() for clause in job.get("if", "").split("&&")} + jobs_in_production_group = [ + job_name for job_name, job in jobs.items() if _concurrency_group(job) == PRODUCTION_CONCURRENCY_GROUP - } - assert clauses_by_job == {"deploy": SUCCESSFUL_MAIN_PUSH_CLAUSES} + ] + assert jobs_in_production_group + for job_name in jobs_in_production_group: + condition = jobs[job_name].get("if", "") + assert _requires_every_clause(condition, SUCCESSFUL_MAIN_PUSH_CLAUSES), job_name + + +SUCCESSFUL_CONCLUSION, PUSH_EVENT, MAIN_HEAD_BRANCH = sorted(SUCCESSFUL_MAIN_PUSH_CLAUSES) + + +@pytest.mark.parametrize( + ("condition", "requires_every_clause"), + [ + pytest.param( + f"{SUCCESSFUL_CONCLUSION} && {PUSH_EVENT} && {MAIN_HEAD_BRANCH}", + True, + id="current wording", + ), + pytest.param( + f"${{{{ {SUCCESSFUL_CONCLUSION} && {PUSH_EVENT} && {MAIN_HEAD_BRANCH} }}}}", + True, + id="expression wrapper", + ), + pytest.param( + f"({MAIN_HEAD_BRANCH} &&\n {PUSH_EVENT}) && {SUCCESSFUL_CONCLUSION}", + True, + id="reordered and grouped", + ), + pytest.param( + f"{SUCCESSFUL_CONCLUSION} && {PUSH_EVENT} && {MAIN_HEAD_BRANCH}" + " && github.repository == 'PolicyEngine/calibration-diagnostics'", + True, + id="extra required clause", + ), + pytest.param( + f"{SUCCESSFUL_CONCLUSION} && {MAIN_HEAD_BRANCH}", + False, + id="missing push clause", + ), + pytest.param( + f"{SUCCESSFUL_CONCLUSION} && {PUSH_EVENT} && {MAIN_HEAD_BRANCH}" + " || github.event.workflow_run.event == 'pull_request'", + False, + id="alternative after or", + ), + pytest.param( + f"!({SUCCESSFUL_CONCLUSION} && {PUSH_EVENT} && {MAIN_HEAD_BRANCH})", + False, + id="negated", + ), + ], +) +def test_clause_check_accepts_rewordings_and_rejects_weaker_conditions( + condition, requires_every_clause +): + assert ( + _requires_every_clause(condition, SUCCESSFUL_MAIN_PUSH_CLAUSES) + is requires_every_clause + ) def test_deployment_uses_exact_project_and_backend_first_order(): From 9792ede212417ad2a7e5e1ecd9513c540a31d923 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:53:09 +0400 Subject: [PATCH 4/4] Accept any deploy condition that still requires a successful main push The check now passes when a job's `if:` is a conjunction that includes the three required clauses, whatever the whitespace, clause order, grouping parentheses, `${{ }}` wrapper or extra `&&`-joined clauses. It still fails when a clause is missing, and it fails on any `||` or `!` (other than `!=`), because either could let the job run without them. Co-Authored-By: Claude Opus 5.5 --- frontend/tests/test_deploy_workflow.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frontend/tests/test_deploy_workflow.py b/frontend/tests/test_deploy_workflow.py index c1a10a39..321321d2 100644 --- a/frontend/tests/test_deploy_workflow.py +++ b/frontend/tests/test_deploy_workflow.py @@ -96,7 +96,19 @@ def test_production_concurrency_group_belongs_to_the_deploy_job_not_the_workflow def _requires_every_clause(condition, required_clauses): - return {clause.strip() for clause in condition.split("&&")} == required_clauses + """Whether `condition` is a conjunction that includes every required clause. + + The `${{ }}` wrapper, whitespace, clause order, grouping parentheses and + extra `&&`-joined clauses are accepted. Any `||` or `!` (other than `!=`) + is rejected, because either could let the job run without the clauses. + """ + expression = " ".join(condition.split()) + if expression.startswith("${{") and expression.endswith("}}"): + expression = expression[3:-2].strip() + if "||" in expression or re.search(r"!(?!=)", expression): + return False + clauses = {clause.strip(" ()") for clause in expression.split("&&")} + return required_clauses <= clauses def test_every_job_in_the_production_group_requires_a_successful_main_push():