From 9daf1e9206d0b6d664cb55f910444f738903775c Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Mon, 12 Oct 2026 05:38:31 +0800 Subject: [PATCH 1/5] ci: bound full Python shards across concurrent runs Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .github/workflows/python-tests.yml | 7 +++ scripts/ci/test_review_gate.py | 84 ++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index e3951701b1..a82606038d 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -423,6 +423,13 @@ jobs: needs: [changes, chat-bundle] if: needs.changes.outputs.python_tests == 'true' runs-on: ubuntu-latest + # Share four long-test lanes across current PRs and other workflow runs. + # Do not replace another candidate's pending shard or cancel its active head. + # Workflow-level concurrency still retires obsolete heads of the same PR. + concurrency: + group: loopx-python-test-shard-${{ matrix.shard }} + queue: max + cancel-in-progress: false # All four shards were still progressing at 83-94% when the 45m job # deadline cancelled run 37916979857. Reserve setup/reporting headroom # without changing test-level deadlines, collection or worker count. diff --git a/scripts/ci/test_review_gate.py b/scripts/ci/test_review_gate.py index 746d2d496f..391378a6a5 100644 --- a/scripts/ci/test_review_gate.py +++ b/scripts/ci/test_review_gate.py @@ -1,6 +1,11 @@ """An intentionally exempt job must be skipped; all required jobs must pass.""" from __future__ import annotations import copy +import os +from pathlib import Path +import re +import subprocess +import tempfile import unittest from review_gate import CHECK_OUTPUT, JOB_OUTPUT, requires_core_tests, verify, verify_checks @@ -89,5 +94,84 @@ def test_backend_flag_cannot_waive_a_full_pr_or_create_client_coverage(self): verifier(value) +class PythonShardAdmissionTests(unittest.TestCase): + """Check the shipped workflow's admission scope and executable gate.""" + + @classmethod + def setUpClass(cls): + cls.workflow = (Path(__file__).resolve().parents[2] + / ".github/workflows/python-tests.yml").read_text() + + def job(self, name): + body = self.workflow.split(f"\n {name}:\n", 1)[1] + return re.split(r"\n [\w-]+:\n", body, maxsplit=1)[0] + + def script(self, job, step): + body = self.job(job).split(f" - name: {step}\n", 1)[1] + body = body.split(" run: |\n", 1)[1] + lines = [] + for line in body.splitlines(): + if line and not line.startswith(" "): + break + lines.append(line[10:]) + return "\n".join(lines) + + def test_required_python_gate_rejects_cancelled_overflow_and_missing_shards(self): + script = self.script("pytest", "Require every upstream check") + for checks in ("success", "failure", "cancelled", "skipped", ""): + for shards in ("success", "failure", "cancelled", "skipped", ""): + result = subprocess.run( + ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + env={**os.environ, "CHECKS_RESULT": checks, "SHARDS_RESULT": shards}, + capture_output=True, + ) + with self.subTest(checks=checks, shards=shards): + self.assertEqual(result.returncode == 0, + checks == shards == "success") + gate = self.job("pytest") + self.assertIn("if: always() && needs.changes.outputs.python_tests == 'true'", gate) + self.assertIn("needs: [changes, checks, test-shard]", gate) + + def test_required_python_gate_reads_every_nonempty_coverage_partition(self): + script = self.script("pytest", "Combine complete coverage and enforce the existing floor") + # Execute the workflow's file guard, before the coverage tool itself. + guard = script.split("python -m coverage combine", 1)[0] + for absent in (None, 1, 2, 3, 4): + for empty in (False, True): + with tempfile.TemporaryDirectory() as directory: + for shard in range(1, 5): + path = Path(directory) / f"coverage-shards/python-coverage-{shard}/.coverage" + path.parent.mkdir(parents=True) + if shard != absent or empty: + path.write_bytes(b"" if shard == absent else b"partition") + result = subprocess.run( + ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", guard], + cwd=directory, capture_output=True, + ) + with self.subTest(absent=absent, empty=empty): + self.assertEqual(result.returncode == 0, absent is None) + + def test_long_shards_share_groups_across_current_heads_without_replacement(self): + job = self.job("test-shard") + admission = re.search(r"\n concurrency:\n((?: [^\n]*\n)+)", job) + self.assertIsNotNone(admission, "long shards need cross-run admission") + fields = dict(re.findall(r"^ ([\w-]+): (.+)$", admission.group(1), re.MULTILINE)) + self.assertEqual(fields.get("queue"), "max") + self.assertEqual(fields.get("cancel-in-progress"), "false") + template = fields["group"] + # A ref/run/PR component would silently restore per-run admission. + self.assertEqual(re.findall(r"\$\{\{\s*(.*?)\s*\}\}", template), ["matrix.shard"]) + groups = {re.sub(r"\$\{\{.*?\}\}", str(shard), template).casefold() + for shard in range(1, 5)} + self.assertEqual(len(groups), 4) + self.assertIn("shard: [1, 2, 3, 4]", job) + self.assertIn("fail-fast: false", job) + root = self.workflow.split("\njobs:\n", 1)[0] + self.assertIn("group: python-tests-${{ github.event_name }}-${{ github.ref }}", root) + self.assertIn("cancel-in-progress: ${{ github.event_name == 'pull_request' || github.event_name == 'push' }}", root) + self.assertIn("permissions:\n contents: read", root) + self.assertNotIn("secrets.", job) + + if __name__ == "__main__": unittest.main() From 4d3536ca0259965b1a6374bb8ee9c8e95481f538 Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Mon, 12 Oct 2026 05:38:31 +0800 Subject: [PATCH 2/5] docs: explain shared Python shard admission and rollback Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- docs/development/testing-and-quality.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/development/testing-and-quality.md b/docs/development/testing-and-quality.md index 4723fb03b7..30d787ccdd 100644 --- a/docs/development/testing-and-quality.md +++ b/docs/development/testing-and-quality.md @@ -596,6 +596,31 @@ Sonar 只复用同一次 run 的 XML，不重复测试、不跨 run 取产物。 Sonar，测试 job 不接收 Sonar secret。触发范围取原有两套 workflow 的并集；纯前端 PR 使用前述豁免，Sonar 配置变更仍全量运行，包括没有 token 的 fork。 +Full Python shards share four repository-wide job concurrency groups, one per +`matrix.shard`, across PR, main, merge-queue and manual runs carrying this +workflow version. Each group admits one running job and uses `queue: max` +to retain up to 100 pending jobs instead of replacing another current +candidate. Workflow-level cancellation still retires an obsolete head of the +same PR. Queue overflow, cancellation and missing coverage remain failures of +the required gate; collection, four partitions, two workers, reports and the +coverage floor stay unchanged. Contributors can wait longer for a shard during +overlapping full runs. This limits long-shard occupancy to four runners; it +does not reserve release capacity or prioritize releases over other jobs or +repositories in the organization's shared hosted pool. Older workflow versions +do not participate. Roll back by removing this job-level concurrency block, +retaining workflow-level cancellation and the required gate. See GitHub's +[concurrency queue contract](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency). + +全量 Python 分片按 `matrix.shard` 共用四个仓库级 job concurrency 组，适用于 +携带本版 workflow 的 PR、main、merge queue 和手动运行。每组允许一个运行中 job， +用 `queue: max` 保留至多 100 个等待 job，不替换其他有效候选。workflow 级取消 +仍清退同 PR 的旧 head；队列溢出、取消和缺失 coverage 仍不能通过必需检查。 +完整 collection、四分片、双 worker、报告和覆盖率门槛保持不变；重叠的全量运行 +会增加贡献者等待分片的时间。此限制只把长分片占用约束到四台 runner，不预留发布 +容量，也不保证发布优先于组织共享 hosted 池中的其他 job 或仓库。旧版 workflow +不参与该限制。回退时移除此 job 的 concurrency 块，保留 workflow 级取消和必需 +检查；队列行为遵循上述 GitHub 官方契约。 + Reproduce one shard locally with `uv run --extra test python -m pytest -v -n 2 --splits 4 --group 1 --splitting-algorithm least_duration --cov=loopx`. Omit the split arguments to run the complete suite locally. 全量本地测试仍省略分片参数即可。 From 2173b4e4fbc9376e7df3c1e3bd793e83ee19b129 Mon Sep 17 00:00:00 2001 From: song Date: Mon, 12 Oct 2026 13:29:39 +0800 Subject: [PATCH 3/5] test(ci): qualify shard admission with the current PR lifecycle gate Signed-off-by: song --- scripts/ci/test_review_gate.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/ci/test_review_gate.py b/scripts/ci/test_review_gate.py index 91f747ecaf..1277ac76a2 100644 --- a/scripts/ci/test_review_gate.py +++ b/scripts/ci/test_review_gate.py @@ -280,8 +280,9 @@ def test_long_shards_share_groups_across_current_heads_without_replacement(self) self.assertIn("shard: [1, 2, 3, 4]", job) self.assertIn("fail-fast: false", job) root = self.workflow.split("\njobs:\n", 1)[0] - self.assertIn("group: python-tests-${{ github.event_name }}-${{ github.ref }}", root) - self.assertIn("cancel-in-progress: ${{ github.event_name == 'pull_request' || github.event_name == 'push' }}", root) + # GateTests executes the current workflow's PR lifecycle expressions, + # including closed events whose ref changes to main. Keep that semantic + # qualification instead of requiring the superseded ref-only spelling. self.assertIn("permissions:\n contents: read", root) self.assertNotIn("secrets.", job) From 0ff848e94e7bd93f7beefca807796e4f9c802ccc Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:52:22 +0800 Subject: [PATCH 4/5] fix(ci): retire obsolete PR runs after synchronize events Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .github/workflows/python-tests.yml | 34 +++++++++++++++--------- docs/development/testing-and-quality.md | 28 +++++++++++++++----- scripts/ci/test_review_gate.py | 35 ++++++++++++++++++++----- 3 files changed, 72 insertions(+), 25 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 5799fa9974..d74a082370 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -5,7 +5,7 @@ on: types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled, closed] # Trusted base-branch code only; never checks out or executes PR content. pull_request_target: - types: [closed] + types: [closed, synchronize] # Inert until the main ruleset enables a merge queue; queue runs are full. merge_group: workflow_dispatch: @@ -43,8 +43,8 @@ env: LOOPX_USAGE_PING: "0" jobs: - cancel-closed-pr: - if: github.event_name == 'pull_request_target' && github.event.action == 'closed' + cancel-obsolete-pr: + if: github.event_name == 'pull_request_target' && (github.event.action == 'closed' || github.event.action == 'synchronize') runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -59,36 +59,46 @@ jobs: const pull_number = context.payload.number; const readPR = async () => (await github.rest.pulls.get({...repo, pull_number})).data; const pr = await readPR(); - if (pr.state !== 'closed') return core.info('PR is open; retain its qualification.'); + const closed = context.payload.action === 'closed'; + if (!closed && context.payload.action !== 'synchronize') return core.info('No obsolete-run cleanup event.'); + if (pr.state !== (closed ? 'closed' : 'open')) return core.info('PR state changed; retain qualification.'); + if (!closed && context.payload.pull_request?.head.sha !== pr.head.sha) { + return core.info('Synchronize head changed; stop cleanup.'); + } const cutoff = Date.parse(pr.closed_at); - if (!pr.head.repo?.id || !pr.head.ref || !Number.isFinite(cutoff)) { - throw new Error('Closed PR has no verifiable source repository, branch or close time.'); + if (!pr.head.repo?.id || !pr.head.ref || !pr.head.sha || (closed && !Number.isFinite(cutoff))) { + throw new Error('PR has no verifiable source repository, branch, head or close time.'); } + const unchanged = current => current.state === pr.state && current.head.sha === pr.head.sha && + current.head.ref === pr.head.ref && current.head.repo?.id === pr.head.repo.id && + (!closed || current.closed_at === pr.closed_at); // Another open PR may use the same source branch against a different base. const open = await github.paginate(github.rest.pulls.list, {...repo, state: 'open', head: pr.head.label, per_page: 100}); - if (open.length) return core.info('Source branch has an open PR; retain its qualification.'); + if (open.some(p => closed || p.number !== pull_number)) { + return core.info('Source branch has another open PR; retain its qualification.'); + } const runs = await github.paginate(github.rest.actions.listWorkflowRuns, {...repo, workflow_id: 'python-tests.yml', event: 'pull_request', branch: pr.head.ref, per_page: 100}); for (const run of runs) { const created = Date.parse(run.created_at); if (run.status === 'completed' || run.head_repository?.id !== pr.head.repo.id || - run.head_branch !== pr.head.ref || !Number.isFinite(created) || created > cutoff) continue; + run.head_branch !== pr.head.ref || !run.head_sha || !Number.isFinite(created) || + (closed ? created > cutoff : run.head_sha === pr.head.sha)) continue; // GitHub can omit PR associations after merge. Only the exact closed head // is safe in that case; superseded heads remain in their concurrency lane. if (run.pull_requests?.length ? !run.pull_requests.some(p => p.number === pull_number) - : run.head_sha !== pr.head.sha) continue; - const current = await readPR(); - if (current.state !== 'closed' || current.head.sha !== pr.head.sha || - current.closed_at !== pr.closed_at) return core.info('PR changed; stop cleanup.'); + : !closed || run.head_sha !== pr.head.sha) continue; + if (!unchanged(await readPR())) return core.info('PR changed; stop cleanup.'); const params = {...repo, run_id: run.id}; try { await github.rest.actions.cancelWorkflowRun(params); await new Promise(resolve => setTimeout(resolve, 1500)); const {data: remaining} = await github.rest.actions.getWorkflowRun(params); if (remaining.status !== 'completed') { + if (!unchanged(await readPR())) return core.info('PR changed; stop force cancellation.'); await github.request('POST /repos/{owner}/{repo}/actions/runs/{run_id}/force-cancel', params); let terminal = false; for (let attempt = 0; attempt < 10; attempt++) { diff --git a/docs/development/testing-and-quality.md b/docs/development/testing-and-quality.md index 405485ed99..3e17c7a7c7 100644 --- a/docs/development/testing-and-quality.md +++ b/docs/development/testing-and-quality.md @@ -175,7 +175,8 @@ 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 gate; open PRs retain the same required jobs and cancellation failures. -A trusted `pull_request_target: closed` job additionally cancels remaining runs +A trusted `pull_request_target` cleanup handles closed and synchronize events. +Closed events cancel remaining runs of this workflow, using job-local `actions: write` without checking out PR code. It verifies the current closed PR, source repository/branch and close time; it preserves reopened PRs, another open PR on that branch, newer runs and @@ -186,8 +187,8 @@ it releases obsolete capacity rather than reserving release capacity or providing a GitHub shared-pool priority guarantee. PR 关闭或合并后，以 PR 编号维持原并发 lane，关闭事件跳过分类与最终门；开放 -PR 的必需检查及取消失败规则不变。来自目标分支的可信 `pull_request_target: -closed` 清理任务仅自身持有 `actions: write`，不检出 PR 代码。它核验当前关闭 +PR 的必需检查及取消失败规则不变。来自目标分支的可信 `pull_request_target` +清理任务处理 closed 与 synchronize 事件（更新规则见下方共享分片说明），仅自身持有 `actions: write`，不检出 PR 代码。它核验当前关闭 状态、源仓库/分支与关闭时间，保留重开的 PR、同源分支上的其他开放 PR、更新 运行及其他仓库；缺少 PR 关联时只处理确切关闭 head。普通取消后仍未结束的 `always()` 汇总使用强制取消 API。清理需要一个短 runner 任务；这会释放过期 @@ -623,8 +624,17 @@ Full Python shards share four repository-wide job concurrency groups, one per `matrix.shard`, across PR, main, merge-queue and manual runs carrying this workflow version. Each group admits one running job and uses `queue: max` to retain up to 100 pending jobs instead of replacing another current -candidate. Workflow-level cancellation still retires an obsolete head of the -same PR. Queue overflow, cancellation and missing coverage remain failures of +candidate. Workflow-level cancellation requests retirement of the same PR's +obsolete head. Trusted base-branch cleanup on `synchronize` also retires its +remaining obsolete runs, including `always()` aggregates that survive ordinary +cancellation. It requires the event's current head, exact PR association, source +repository and branch, and rechecks the current head before ordinary and force +cancellation. It preserves another open PR sharing that branch. Current-head +runs, unassociated open-PR runs and ordinary required gate predicates remain +unchanged: cancelled, skipped or incomplete dependencies cannot become a green +aggregate. This adds one short trusted cleanup job per update without checkout, +PR-code execution or new permissions on qualification jobs. Older workflow +versions on the base branch retain only closed-event cleanup. Queue overflow, cancellation and missing coverage remain failures of the required gate; collection, four partitions, two workers, reports and the coverage floor stay unchanged. Contributors can wait longer for a shard during overlapping full runs. This limits long-shard occupancy to four runners; it @@ -637,7 +647,13 @@ retaining workflow-level cancellation and the required gate. See GitHub's 全量 Python 分片按 `matrix.shard` 共用四个仓库级 job concurrency 组，适用于 携带本版 workflow 的 PR、main、merge queue 和手动运行。每组允许一个运行中 job， 用 `queue: max` 保留至多 100 个等待 job，不替换其他有效候选。workflow 级取消 -仍清退同 PR 的旧 head；队列溢出、取消和缺失 coverage 仍不能通过必需检查。 +请求清退同 PR 的旧 head；可信 base 分支的 `synchronize` 清理还会取消遗留 +旧 run，包括普通取消后仍继续的 `always()` 汇总。清理要求事件中的当前 head、 +精确 PR 关联、源仓库和分支，在普通及强制取消前重新读取 head；同分支另一 open +PR、当前 head 和无关联 run 均被保护。必过汇总条件不变，取消、跳过或缺失依赖 +不能变绿。每次更新增加一个短清理 job，不 checkout 或执行 PR 代码，不增加普通 +验证 job 的权限；base 分支旧版 workflow 仍只清理 closed 事件。队列溢出、取消 +和缺失 coverage 仍不能通过必需检查。 完整 collection、四分片、双 worker、报告和覆盖率门槛保持不变；重叠的全量运行 会增加贡献者等待分片的时间。此限制只把长分片占用约束到四台 runner，不预留发布 容量，也不保证发布优先于组织共享 hosted 池中的其他 job 或仓库。旧版 workflow diff --git a/scripts/ci/test_review_gate.py b/scripts/ci/test_review_gate.py index 7129c4feac..44b16156f9 100644 --- a/scripts/ci/test_review_gate.py +++ b/scripts/ci/test_review_gate.py @@ -43,6 +43,7 @@ def test_closed_pr_cancels_same_lane_and_skips_qualification(self): Loader=yaml.BaseLoader, ) self.assertIn("closed", workflow["on"]["pull_request"]["types"]) + self.assertEqual(workflow["on"]["pull_request_target"]["types"], ["closed", "synchronize"]) concurrency = workflow["concurrency"] jobs = workflow["jobs"] @@ -72,11 +73,12 @@ def lane(action, **kwargs): for name in ("changes", "merge-gate"): self.assertTrue(expression(jobs[name]["if"], "", event=event)) self.assertEqual([name for name, job in jobs.items() if not job.get("needs")], - ["cancel-closed-pr", "changes"]) + ["cancel-obsolete-pr", "changes"]) for name in ("changes", "merge-gate"): self.assertFalse(expression(jobs[name]["if"], "closed", event="pull_request_target")) - cleanup = jobs["cancel-closed-pr"] + cleanup = jobs["cancel-obsolete-pr"] self.assertTrue(expression(cleanup["if"], "closed", event="pull_request_target")) + self.assertTrue(expression(cleanup["if"], "synchronize", event="pull_request_target")) self.assertFalse(expression(cleanup["if"], "closed")) self.assertFalse(expression(cleanup["if"], "reopened", event="pull_request_target")) self.assertNotEqual(lane("closed", event="pull_request_target"), @@ -91,12 +93,12 @@ def lane(action, **kwargs): self.assertIn("needs.changes.outputs.", job["if"]) self.assertIn("== 'true'", job["if"]) - def test_trusted_cleanup_executes_only_obsolete_closed_pr_runs(self): + def test_trusted_cleanup_retires_obsolete_heads_without_skipping_required_gates(self): workflow = yaml.load( (Path(__file__).resolve().parents[2] / ".github/workflows/python-tests.yml").read_text(), Loader=yaml.BaseLoader, ) - script = workflow["jobs"]["cancel-closed-pr"]["steps"][0]["with"]["script"] + script = workflow["jobs"]["cancel-obsolete-pr"]["steps"][0]["with"]["script"] # Execute the shipped transport script, independently supplying legal and # illegal lifecycle fixtures. No token or untrusted PR code is involved. harness = r""" @@ -109,9 +111,10 @@ def test_trusted_cleanup_executes_only_obsolete_closed_pr_runs(self): const old = {id:1, status:'queued', head_repository:{id:7}, head_branch:'fix', head_sha:'head', created_at:'2026-01-01T00:00:00Z', pull_requests:[]}; async function run({first=pr, current=first, open=[], runs=[old], ordinaryDone=false, - conflict=false, stuck=false, failure=false}={}) { + conflict=false, stuck=false, failure=false, action='closed', + eventHead=first.head.sha, beforeForce=current}={}) { const cancelled=[], forced=[], queries=[]; let reads=0; - const github = {rest:{pulls:{get:async()=>({data:reads++ ? current : first}), list:'pulls'}, + const github = {rest:{pulls:{get:async()=>({data:reads++ ? (reads > 2 ? beforeForce : current) : first}), list:'pulls'}, actions:{listWorkflowRuns:'runs', cancelWorkflowRun:async p=>{cancelled.push(p.run_id); if(conflict || failure) throw Object.assign(new Error('provider failure'), {status:conflict ? 409 : 403});}, @@ -120,7 +123,7 @@ def test_trusted_cleanup_executes_only_obsolete_closed_pr_runs(self): paginate:async (method,params)=>{queries.push([method,params]); return method==='pulls' ? open : runs;}, request:async (route,p)=>{assert.equal(route, 'POST /repos/{owner}/{repo}/actions/runs/{run_id}/force-cancel'); forced.push(p.run_id);}}; - await execute(github, {repo:{owner:'owner',repo:'repo'},payload:{number:17}}, + await execute(github, {repo:{owner:'owner',repo:'repo'},payload:{number:17,action,pull_request:{head:{sha:eventHead}}}}, {info:()=>{}}, fn=>fn()); return {cancelled,forced,queries}; } @@ -139,6 +142,24 @@ def test_trusted_cleanup_executes_only_obsolete_closed_pr_runs(self): {current:{...pr,head:{...pr.head,sha:'new'}}}, {open:[{number:18}]}]) { assert.deepEqual((await run(fixture)).cancelled,[]); } +const active = {...pr,state:'open',closed_at:null}; +const associatedOld = {...old,head_sha:'previous',pull_requests:[{number:17}]}; +const updated = await run({action:'synchronize',first:active,open:[{number:17}], + runs:[associatedOld, {...associatedOld,id:2,head_sha:'head'}, + {...associatedOld,id:3,pull_requests:[]}, {...associatedOld,id:4,pull_requests:[{number:18}]}, + {...associatedOld,id:5,head_repository:{id:8}}, {...associatedOld,id:6,head_branch:'another'}, + {...associatedOld,id:7,head_sha:''}, {...associatedOld,id:8,status:'completed'}]}); +assert.deepEqual(updated.cancelled,[1]); assert.deepEqual(updated.forced,[1]); +for (const fixture of [{eventHead:'stale'}, {current:pr}, + {current:{...active,head:{...active.head,sha:'new'}}}, {open:[{number:18}]}, + {current:{...active,head:{...active.head,repo:{id:8}}}}, + {current:{...active,head:{...active.head,ref:'another'}}}]) { + assert.deepEqual((await run({action:'synchronize',first:active,runs:[associatedOld],...fixture})).cancelled,[]); +} +const changedDuringCancel = await run({action:'synchronize',first:active,runs:[associatedOld], + beforeForce:{...active,head:{...active.head,sha:'previous'}}}); +assert.deepEqual(changedDuringCancel.cancelled,[1]); assert.deepEqual(changedDuringCancel.forced,[]); +assert.deepEqual((await run({action:'reopened',first:active})).cancelled,[]); assert.deepEqual((await run({ordinaryDone:true})).forced,[]); assert.deepEqual((await run({conflict:true})).forced,[]); await assert.rejects(run({stuck:true}),/has not completed/); From 179ad1d706ec0451f643a0ae1ccc22369aab949d Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:36:58 +0800 Subject: [PATCH 5/5] Recheck shared PR branch before obsolete-run cancellation Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .github/workflows/python-tests.yml | 15 +++++++---- docs/development/testing-and-quality.md | 9 +++++-- scripts/ci/test_review_gate.py | 34 ++++++++++++++++++++++--- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index d74a082370..85d9cd570e 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -73,9 +73,14 @@ jobs: current.head.ref === pr.head.ref && current.head.repo?.id === pr.head.repo.id && (!closed || current.closed_at === pr.closed_at); // Another open PR may use the same source branch against a different base. - const open = await github.paginate(github.rest.pulls.list, - {...repo, state: 'open', head: pr.head.label, per_page: 100}); - if (open.some(p => closed || p.number !== pull_number)) { + const hasOtherOpenPR = async (head = pr.head.label) => (await github.paginate(github.rest.pulls.list, + {...repo, state: 'open', head, per_page: 100})) + .some(p => closed || p.number !== pull_number); + const cancellationScopeUnchanged = async () => { + const current = await readPR(); + return unchanged(current) && !(await hasOtherOpenPR(current.head.label)); + }; + if (await hasOtherOpenPR()) { return core.info('Source branch has another open PR; retain its qualification.'); } const runs = await github.paginate(github.rest.actions.listWorkflowRuns, @@ -91,14 +96,14 @@ jobs: if (run.pull_requests?.length ? !run.pull_requests.some(p => p.number === pull_number) : !closed || run.head_sha !== pr.head.sha) continue; - if (!unchanged(await readPR())) return core.info('PR changed; stop cleanup.'); + if (!(await cancellationScopeUnchanged())) return core.info('PR or source branch changed; stop cleanup.'); const params = {...repo, run_id: run.id}; try { await github.rest.actions.cancelWorkflowRun(params); await new Promise(resolve => setTimeout(resolve, 1500)); const {data: remaining} = await github.rest.actions.getWorkflowRun(params); if (remaining.status !== 'completed') { - if (!unchanged(await readPR())) return core.info('PR changed; stop force cancellation.'); + if (!(await cancellationScopeUnchanged())) return core.info('PR or source branch changed; stop force cancellation.'); await github.request('POST /repos/{owner}/{repo}/actions/runs/{run_id}/force-cancel', params); let terminal = false; for (let attempt = 0; attempt < 10; attempt++) { diff --git a/docs/development/testing-and-quality.md b/docs/development/testing-and-quality.md index 3e17c7a7c7..b1d92fb182 100644 --- a/docs/development/testing-and-quality.md +++ b/docs/development/testing-and-quality.md @@ -182,7 +182,10 @@ It verifies the current closed PR, source repository/branch and close time; it preserves reopened PRs, another open PR on that branch, newer runs and foreign repositories. A missing run-to-PR association requires the exact closed head. After ordinary cancellation, it uses GitHub's force-cancel endpoint for -nonterminal `always()` aggregates. The cleanup needs one short runner job; +nonterminal `always()` aggregates. Before either cancellation stage it rereads +both the target PR and open PRs sharing its source branch; a new sibling or +unavailable lookup prevents the effect. A fresh retry can resume after the +branch is no longer shared. The cleanup needs one short runner job; it releases obsolete capacity rather than reserving release capacity or providing a GitHub shared-pool priority guarantee. @@ -191,7 +194,9 @@ PR 的必需检查及取消失败规则不变。来自目标分支的可信 `pul 清理任务处理 closed 与 synchronize 事件（更新规则见下方共享分片说明），仅自身持有 `actions: write`，不检出 PR 代码。它核验当前关闭 状态、源仓库/分支与关闭时间，保留重开的 PR、同源分支上的其他开放 PR、更新 运行及其他仓库；缺少 PR 关联时只处理确切关闭 head。普通取消后仍未结束的 -`always()` 汇总使用强制取消 API。清理需要一个短 runner 任务；这会释放过期 +`always()` 汇总使用强制取消 API。普通及强制取消前都重读目标 PR 和同源分支的 +开放 PR；新出现的共享分支 PR 或读取失败会阻止副作用，分支不再共享后可重新 +触发清理。清理需要一个短 runner 任务；这会释放过期 容量，不等于为发布预留容量或设置 GitHub 共享池优先级。 The [job exemption policy](ci-impact-selection.md) additionally permits pure diff --git a/scripts/ci/test_review_gate.py b/scripts/ci/test_review_gate.py index 44b16156f9..3f7e5249fc 100644 --- a/scripts/ci/test_review_gate.py +++ b/scripts/ci/test_review_gate.py @@ -112,15 +112,24 @@ def test_trusted_cleanup_retires_obsolete_heads_without_skipping_required_gates( head_sha:'head', created_at:'2026-01-01T00:00:00Z', pull_requests:[]}; async function run({first=pr, current=first, open=[], runs=[old], ordinaryDone=false, conflict=false, stuck=false, failure=false, action='closed', - eventHead=first.head.sha, beforeForce=current}={}) { + eventHead=first.head.sha, beforeForce=current, + beforeCancelOpen=open, beforeForceOpen=beforeCancelOpen, + refreshFailure=false}={}) { const cancelled=[], forced=[], queries=[]; let reads=0; + let branchScope=open, openReads=0; const github = {rest:{pulls:{get:async()=>({data:reads++ ? (reads > 2 ? beforeForce : current) : first}), list:'pulls'}, actions:{listWorkflowRuns:'runs', cancelWorkflowRun:async p=>{cancelled.push(p.run_id); if(conflict || failure) - throw Object.assign(new Error('provider failure'), {status:conflict ? 409 : 403});}, + throw Object.assign(new Error('provider failure'), {status:conflict ? 409 : 403}); + branchScope=beforeForceOpen;}, getWorkflowRun:async p=>({data:{status:(conflict || ordinaryDone || (forced.includes(p.run_id) && !stuck)) ? 'completed' : 'queued'}})}}, - paginate:async (method,params)=>{queries.push([method,params]); return method==='pulls' ? open : runs;}, + paginate:async (method,params)=>{queries.push([method,params]); + if(method==='pulls') { + if(openReads++ && refreshFailure) throw new Error('branch read unavailable'); + return branchScope; + } + branchScope=beforeCancelOpen; return runs;}, request:async (route,p)=>{assert.equal(route, 'POST /repos/{owner}/{repo}/actions/runs/{run_id}/force-cancel'); forced.push(p.run_id);}}; await execute(github, {repo:{owner:'owner',repo:'repo'},payload:{number:17,action,pull_request:{head:{sha:eventHead}}}}, @@ -159,6 +168,25 @@ def test_trusted_cleanup_retires_obsolete_heads_without_skipping_required_gates( const changedDuringCancel = await run({action:'synchronize',first:active,runs:[associatedOld], beforeForce:{...active,head:{...active.head,sha:'previous'}}}); assert.deepEqual(changedDuringCancel.cancelled,[1]); assert.deepEqual(changedDuringCancel.forced,[]); +for (const fixture of [{beforeCancelOpen:[{number:18}]}, + {action:'synchronize',first:active,open:[{number:17}],runs:[associatedOld], + beforeCancelOpen:[{number:17},{number:18}]}]) { + assert.deepEqual((await run(fixture)).cancelled,[]); +} +for (const fixture of [{beforeForceOpen:[{number:18}]}, + {action:'synchronize',first:active,open:[{number:17}],runs:[associatedOld], + beforeForceOpen:[{number:17},{number:18}]}]) { + const changedBranch = await run(fixture); + assert.deepEqual(changedBranch.cancelled,[1]); + assert.deepEqual(changedBranch.forced,[]); +} +await assert.rejects(run({refreshFailure:true}),/branch read unavailable/); +// A fresh retry after the sibling closes can retire the original run. +assert.deepEqual((await run()).forced,[1]); +const movedOwner = await run({current:{...pr,head:{...pr.head,label:'moved:fix'}}}); +assert.deepEqual(movedOwner.forced,[1]); +assert.ok(movedOwner.queries.filter(([method])=>method==='pulls').slice(1) + .every(([,params])=>params.head==='moved:fix')); assert.deepEqual((await run({action:'reopened',first:active})).cancelled,[]); assert.deepEqual((await run({ordinaryDone:true})).forced,[]); assert.deepEqual((await run({conflict:true})).forced,[]);