From edc5cd1e4d6c48b3848f144aaae738b5f2681938 Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:21:44 +0800 Subject: [PATCH] fix(ci): stop aggregate runners for cancelled main pushes Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .github/workflows/python-tests.yml | 9 +++++---- docs/development/testing-and-quality.md | 17 ++++++++++++++++ scripts/ci/test_review_gate.py | 6 ++++-- tests/test_ci_resource_policy.py | 27 +++++++++++++++++++++---- 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index dfcb4926bf..46dff876fb 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -504,7 +504,8 @@ jobs: checks: # Preserve the required check name while exposing independent failure lanes. - if: always() && needs.changes.outputs.core_tests == 'true' + # Cancelled main pushes must not retain runner-backed aggregate work. + if: always() && needs.changes.outputs.core_tests == 'true' && (github.event_name != 'push' || !cancelled()) needs: [changes, kernel-static-checks, typescript-coverage, dashboard-acceptance, chat-bundle-browser] runs-on: ubuntu-latest timeout-minutes: 2 @@ -658,7 +659,7 @@ jobs: pytest: # Keep the required check name; a skipped/failed shard must not turn it green. - if: always() && needs.changes.outputs.python_tests == 'true' + if: always() && needs.changes.outputs.python_tests == 'true' && (github.event_name != 'push' || !cancelled()) needs: [changes, checks, test-shard] runs-on: ubuntu-latest timeout-minutes: 10 @@ -795,7 +796,7 @@ jobs: stage2c-correctness-e2e: # Preserve the public check name and reject failed, cancelled or skipped lanes. - if: always() && needs.changes.outputs.stage2c_tests == 'true' + if: always() && needs.changes.outputs.stage2c_tests == 'true' && (github.event_name != 'push' || !cancelled()) needs: [changes, stage2c-suite] runs-on: ubuntu-latest timeout-minutes: 2 @@ -943,7 +944,7 @@ jobs: merge-gate: # Open PRs always receive an outcome; closed PRs only cancel their old lane. - if: always() && github.event_name != 'pull_request_target' && (github.event_name != 'pull_request' || github.event.action != 'closed') + if: always() && github.event_name != 'pull_request_target' && (github.event_name != 'pull_request' || github.event.action != 'closed') && (github.event_name != 'push' || !cancelled()) needs: [changes, checks, pytest, node-minimum-compatibility, stage2c-correctness-e2e, windows-powershell, presentation] runs-on: ubuntu-latest timeout-minutes: 3 diff --git a/docs/development/testing-and-quality.md b/docs/development/testing-and-quality.md index e8e808a3b8..07b7e9882d 100644 --- a/docs/development/testing-and-quality.md +++ b/docs/development/testing-and-quality.md @@ -170,6 +170,23 @@ Changes to the classifier or workflow need both code-path and documentation-only qualification. Keep required check names stable and never require a workflow-level path-filtered check that cannot report on every PR. +Cancelled `main` push workflows skip the runner-backed `checks`, `pytest`, +`stage2c-correctness-e2e` and `merge-gate` aggregates. Previously their job-level +`always()` conditions kept requesting runners after cancellation. The guard +uses GitHub's [`cancelled()` status](https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#status-check-functions) +only for push events: an uncancelled main run still aggregates failed upstream +work, and PR, nightly, manual and merge-group gate conditions retain their +existing behavior. Evidence-upload steps keep `always()`. A cancelled main +cohort is incomplete qualification, never a successful full run; this change +does not reserve capacity or establish a release-priority guarantee. + +已取消的 `main` push 工作流跳过需申请 runner 的 `checks`、`pytest`、 +`stage2c-correctness-e2e` 和 `merge-gate` 汇总；此前 job 级 `always()` 在取消后 +仍会申请 runner。新条件仅对 push 事件检查 `cancelled()`：未取消的主干运行仍汇总 +上游失败，PR、夜间、手动和合并队列的门禁条件保留原行为。证据上传步骤继续使用 +`always()`。已取消主干候选仍是未完成资格，不能报告全量成功；本改动不预留容量， +也不保证发布优先级。 + Closing or merging a PR retires its outstanding Python qualification. A PR number keeps the same concurrency lane when the merged close event changes `github.ref` to the base branch. Closed events skip classification and the final diff --git a/scripts/ci/test_review_gate.py b/scripts/ci/test_review_gate.py index ff20a4854b..0d89a5cf27 100644 --- a/scripts/ci/test_review_gate.py +++ b/scripts/ci/test_review_gate.py @@ -54,8 +54,9 @@ def expression(value, action, *, number=17, event="pull_request"): github = SimpleNamespace(event_name=event, ref=ref, event=SimpleNamespace( action=action, number=number, pull_request=None)) return eval(value.strip().removeprefix("${{").removesuffix("}}").strip() - .replace("&&", "and").replace("||", "or"), + .replace("&&", "and").replace("||", "or").replace("!cancelled()", "not cancelled()"), {"__builtins__": {}}, {"github": github, "always": lambda: True, + "cancelled": lambda: False, "format": lambda template, number: template.format(number)}) def lane(action, **kwargs): @@ -283,7 +284,8 @@ def test_required_python_gate_rejects_cancelled_overflow_and_missing_shards(self self.assertEqual(result.returncode == 0, checks == shards == "success") gate = self.workflow["jobs"]["pytest"] - self.assertEqual(gate["if"], "always() && needs.changes.outputs.python_tests == 'true'") + self.assertEqual(gate["if"], "always() && needs.changes.outputs.python_tests == 'true' " + "&& (github.event_name != 'push' || !cancelled())") self.assertEqual(gate["needs"], ["changes", "checks", "test-shard"]) def test_required_python_gate_reads_every_nonempty_coverage_partition(self): diff --git a/tests/test_ci_resource_policy.py b/tests/test_ci_resource_policy.py index 9cd4dfacac..a25ce64845 100644 --- a/tests/test_ci_resource_policy.py +++ b/tests/test_ci_resource_policy.py @@ -22,21 +22,23 @@ def event(name: str, *, pr: int = 101, tag: str = "", backend: bool = True) -> d ref=f"refs/pull/{pr}/merge" if name == "pull_request" else "refs/heads/main", # Pull-request webhooks expose number at the payload root as well # as inside pull_request; the shared concurrency owner uses both. - event=SimpleNamespace(number=pr, pull_request=SimpleNamespace(number=pr), release=SimpleNamespace(tag_name=tag)), + event=SimpleNamespace(action="opened", number=pr, pull_request=SimpleNamespace(number=pr), release=SimpleNamespace(tag_name=tag)), ), "inputs": SimpleNamespace(tag=tag), - "needs": SimpleNamespace(changes=SimpleNamespace(outputs=SimpleNamespace(backend_tests=str(backend).lower()))), + "needs": SimpleNamespace(changes=SimpleNamespace(outputs=SimpleNamespace(**{ + key: str(backend).lower() for key in ("backend_tests", "core_tests", "python_tests", "stage2c_tests") + }))), } def expression(value: str | bool, context: dict) -> str | bool: - """Evaluate the boolean/string subset used by these concurrency conditions.""" + """Evaluate the boolean/string subset used by these CI admission conditions.""" if isinstance(value, bool): return value def evaluate(source: str): return eval( # noqa: S307 - repository-owned expressions, no builtins. - source.strip().replace("&&", " and ").replace("||", " or "), + re.sub(r"!(?!=)", " not ", source.strip()).replace("&&", " and ").replace("||", " or "), {"__builtins__": {}, "format": lambda template, *args: template.format(*args)}, context, ) @@ -84,6 +86,23 @@ def test_main_supersession_does_not_cancel_nightly_or_manual_full_qualification( assert [expression(concurrency["cancel-in-progress"], context) for context in contexts] == [True, True, False, False, False] +@pytest.mark.parametrize("job_id", ["checks", "pytest", "stage2c-correctness-e2e", "merge-gate"]) +@pytest.mark.parametrize("name", ["push", "pull_request", "pull_request_target", "schedule", "workflow_dispatch", "merge_group"]) +@pytest.mark.parametrize("cancelled", [False, True]) +@pytest.mark.parametrize("planned", [False, True]) +def test_cancelled_main_aggregates_stop_while_other_event_gates_remain_required( + job_id: str, name: str, cancelled: bool, planned: bool, +) -> None: + context = event(name, backend=planned) + # always() also models failed upstream work: these jobs must still run + # their real rejection commands unless the main push itself is cancelled. + context.update(always=lambda: True, cancelled=lambda: cancelled) + expected = planned if job_id != "merge-gate" else name != "pull_request_target" + if name == "push" and cancelled: + expected = False + assert expression(workflow("python-tests")["jobs"][job_id]["if"], context) is expected + + @pytest.mark.parametrize("name", ["pull_request", "pull_request_target"]) def test_python_pr_supersession_uses_the_payload_number(name: str) -> None: concurrency = workflow("python-tests")["concurrency"]