ci: restore the post-merge push trigger on main - #10345
Conversation
`ci.yml` dropped its `push` trigger on the premise that the merge queue validates the merged tree before it lands. No merge queue is enabled — no queue run since 2026-07-02 — so nothing validated that tree on either side of the merge: `main` had no gate before a merge, and no check faster than the ~40-minute E2E after one. Accept `push` on `main` in the `test` job alone. Every other job keeps its existing gate, so a post-merge run is exactly Classify PR + Test, and a nightly still resolves to exactly the two platform lanes. The expensive, PR-shaped steps inside `test` — the no-AK integration gate, the checkout-head verifier and the test reporter — already carry their own pull_request / merge_group gates, so the push lane is lint, static analysis and unit tests. Route the lane to the ECS pool: a push to `main` is the most trusted context there is, and Test is the ten-plus-minute job that should not spend a scarce hosted Linux runner on every merge. classify_pr's own `runs-on` keeps no push arm by design — it is the canonical association-routing text that sdk-java.yml and serve-ab.yml mirror, and a push carries no author association to route on — so the two halves disagree on push deliberately; a new routing test pins that asymmetry so a later "drift fix" has to read the reason first.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
第一轮
|
|
Thanks for the PR! Template looks good ✓ Problem: real and verified, not theoretical. Direction: aligned. This restores the fast post-merge signal without gating merges, changing required checks, or touching the merge queue — a well-scoped CI reliability fix, and it documents its own exit condition (remove the trigger again once a merge queue actually exists). CHANGELOG: N/A — CI infrastructure, no user-visible surface. Size: no core paths touched. 50 production lines in Approach: minimal and on point — re-enable the trigger for Risk: no elevated risk signals — none of the changed files match the revert-correlated paths. One note for the reviewer: this changes the CI pipeline itself, so the load-bearing part of the review is the job-versus-event audit — that belongs to the code review and follows in the next comment. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 真实存在且已核实,不是理论性问题。 方向: 对齐。本改动恢复了快速的合并后信号,既不做合并门禁,也不改变 required checks 或合并队列——是一个范围恰当的 CI 可靠性修复,并且写明了自己的退出条件(一旦真正启用合并队列就再次移除该触发器)。CHANGELOG:不适用——纯 CI 基础设施,无用户可见面。 规模: 未触及核心路径。 方案: 最小且聚焦——仅为 风险: 无升级风险信号——改动的文件均不匹配与 revert 相关的高风险路径。给评审者提个醒:本改动动的是 CI 流水线本身,因此评审的关键在于 job × 事件矩阵的审计——见下一条评论。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, the plausible options were: a dedicated minimal post-merge workflow (duplicates the lint/unit definitions and drifts from I verified that audit against the actual workflow files rather than the description:
No correctness blockers, no convention violations, nothing to split out. Test evidence — the PR's own CIUnattended run, so the gate never executes PR code; the evidence below is the PR's own CI, read via the API. One note on the run history: the first head ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The long pole is the Sandboxed lanes do not apply here: 中文说明代码审查在读 diff 之前,合理的选择有三个:单独建一个最小的合并后 workflow(会重复 lint/单元测试定义,并随时间与 我没有采信描述,而是对照实际 workflow 文件核实了这份审计:
无正确性阻塞项、无约定违规、无可拆分内容。 测试证据 —— 来自 PR 自身的 CI无人值守运行,门禁从不执行 PR 代码;以下证据是经由 API 读取的 PR 自身 CI。关于运行历史需要说明:第一个 head( 耗时最长的是 沙箱通道在此不适用: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, fully verified change; the only open item is the Test suite still running on the current head. The motivation holds up under scrutiny: the removal's premise (a merge queue validates the merged tree) is provably false on this repository — no queue run since 2026-07-02 and no The first head went red for a reason outside this diff — the branch predated Verdict: approve — approval deferred until CI lands green on 中文说明信心:4/5 —— 干净、最小、已充分核实的改动;唯一未决事项是当前 head 上的 Test 套件仍在运行。 动机经得起推敲:当初移除的理由(合并队列会验证合并树)在本仓库已被证明不成立——自 2026-07-02 以来没有任何队列运行, 第一个 head 变红的原因在本 diff 之外——分支切出早于 结论:批准——批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Eight findings from the review pass, all confirmed against the tree. R1-1 (Critical): the `test` job's checkout `ref:` fell through to `github.ref` on push — `refs/heads/main` resolved at fetch time, a moving tip — while the check run attaches to `github.sha`. The lane could validate a tree it did not report on, and nothing detected it: the checkout-head verifier is gated to pull_request / merge_group and its ancestor check passes for a newer tip regardless. Regressions would still be caught (the tested tree is a descendant), but attribution breaks — red on a clean commit, autofix filed against the wrong merge. Pin push runs to `github.sha`, mirroring the merge_group arm; `e2e.yml` gets the same guarantee from actions/checkout's default. The five other checkout copies sit in jobs that exclude push. R1-5: the concurrency group was keyed on ref alone, so a post-merge run shared `Qwen Code CI-refs/heads/main` with the nightly (60-minute lanes, neither cancelling on main) and with any PR from a fork branch named literally `refs/heads/main` — a legal ref name that makes the group byte-identical while that run cancels in progress. Scope the group by event, as `e2e.yml` already does. R1-6: `WORKFLOW_SIZE_BASE_SHA` was empty on push, so both size enforcers failed closed there while PRs stayed green on leniency — the next stale baseline would redden every merge with an error naming a PR that does not exist. Add `github.event.before`; `github.sha` stays rejected because workflow_dispatch would compare a file against itself. R1-4: the coverage upload ran on push although its only consumer, post_coverage_comment, is gated to pull_request and nothing else reads the artifact. R1-2: the push lane's YAML wiring had no test witness — the routing tests drive pick_runner's shell directly and stay green if the trigger or either job gate loses its push arm. Add assertions covering the trigger, both accepting jobs, the exclusivity of every other job, and the three fixes above. Each is mutation-checked: removing any one fix reddens exactly one test. R1-3, R1-7 and R1-8 are three comments the trigger falsified: the failure watcher's "SCHEDULED run only" header, af-025's "ci.yml has no push trigger" premise, and this PR's own "only `test` accepts this trigger", which contradicted the classify_pr arm added in the same diff. The added prose put ci.yml over its size baseline; trimmed back under the allowance rather than raising the baseline.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1d": none — the walk finished inside budget..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1d":none — the walk finished inside budget.。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Five follow-up findings, all on the guards added in the previous commit rather than on the lane itself.
R1-2 (fix-induced): `test`'s push admission was pinned only negatively — no `!= 'push'` — so rewriting `test.if` into allowlist form drops push while every guard stays green: the exclusivity helper skips `test` because it is an expected push job, and the routing tests inject EVENT_NAME=push straight into pick_runner's shell, bypassing the job gate entirely. On a real merge the run would conclude `success` with the lane silently gone, and the failure watcher — gated on `conclusion == 'failure'` — would file nothing. Require the gate to stay exclusion-shaped.
R2-1: the push exclusivity loop was a hand-copy of the nightly one and the two had already diverged — the mirror's allowlist gained `schedule` while the original never gained `push` — so a push-only job would fail the nightly assertion with a message naming the wrong event. Replaced both with one parameterized helper, which also restores the `not.toBe('')` assertion the copy dropped.
R2-2: the concurrency assertion pinned only the added `github.event_name` token. Flipping `cancel-in-progress` to `true` was measured invisible to all 1854 test:scripts tests, while consecutive merges would then cancel each other — the history this repo already recorded once, when 67 of 100 push runs were cancelled and only 25 reported. Pin all three components, the way e2e-workflow.test.js pins its own.
R2-3: the group's pull_request half was still keyed on `head_ref` alone, so same-named branches across forks — and a fork's default branch is `main`, the commonest head_ref there is — share one group and cancel each other's in-progress runs. Add the source-repo term, matching security-checks.yml and serve-ab.yml. Supersede within a PR is preserved: one branch backs at most one open PR per fork.
R2-4: the trigger was pinned by `branches` only; a sibling `tags:` key would widen the lane while the suite stayed green. On a tag-creation push `github.event.before` is all zeros, so WORKFLOW_SIZE_BASE_SHA is unresolvable and both size enforcers take their strict arm, while the failure watcher's `head_branch == 'main'` filter files nothing — a silent red on release day. Pin the whole trigger object.
Mutation-checked: each of the five reddens exactly the intended assertion, and the extracted helper now reports the correct event for a push-only job (verified by injecting a probe job — the nightly assertion passes, the push one fails naming push).
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Convergence: round 3 posted 7 inline comment(s), 7 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: scripts/tests/ci-platform-lanes.test.js (findings in rounds 1, 2; 5 more now); .github/workflows/ci.yml (findings in round 2; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛情况:第 3 轮发布了 7 条行内评论,其中 7 条是首次提出;上一轮发布了 5 条(其中 5 条首次提出)。发现反复回到同一批文件:scripts/tests/ci-platform-lanes.test.js(第 1、2 轮已出过发现,本轮又有 5 条);.github/workflows/ci.yml(第 2 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
Round 3 raised seven findings, all Suggestion, and they are all instances of one root cause the review itself named: the guards asserted on substrings, and a substring cannot see a connective. Rather than adding one more substring per mutation, this replaces the two mechanisms behind them. Gate reachability is now decided by EVALUATING each job's `if` for the event instead of looking for tokens in it — the substitute-then-evaluate technique `.github/scripts/ci-runner-routing.test.mjs` already uses on `runs-on`. That closes R2-1's connective blindness (an `&&`→`||` flip inside an excluded job; an allowlisted equality paired with an event-neutral disjunct under `||`) without the recursive boolean parser the suggestion proposed: a hand-written expression parser is a new fail-open surface of its own, whereas evaluation computes the answer rather than pattern-matching the question. Any context term the substitutions do not model now throws instead of reading as excluded. The helper also asserts the positive direction — that the jobs which must run on an event do — which the token version never did, and which is precisely the "a lane nobody is told about" failure this file's header says it exists to prevent. Every security-relevant gate is pinned as a whole literal rather than by substring: `classify_pr.if`, `test.if`, the checkout `ref:` expression, the coverage step's `if`, and both halves of the concurrency block. One shape closes R3-3, R1-2, R3-4 and R2-2 together — connector flips, added exclusions, deleted clauses and reordering all change the literal. R3-2: the trigger comment claimed the test reporter is restricted to pull_request / merge_group. It carries no event gate at all; it is excluded by a same-repo clause that also keeps fork PRs out, so normalising it to the documented allowlist would run dorny/test-reporter on fork events that lack `checks: write`. Reworded per step, with that trap named. R3-1: bump `.size-baseline` for ci.yml to 85336. The previous two rounds trimmed comments to stay under the allowance, which left 62 bytes of headroom and charged the next PR for growth this one introduced. The baseline file's own header says growth is deliberate and asks for the number to be bumped with a reason — the growth here is the per-finding rationale the review asked for, so it is recorded rather than squeezed out. Mutation matrix across all three rounds: 22 mutations, none escaping — the seven this round probed as surviving now redden, and the twelve from rounds 1 and 2 still do.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Convergence: round 4 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 7 (7 new). Findings keep coming back to the same files: scripts/tests/ci-platform-lanes.test.js (findings in rounds 1, 2, 3; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛情况:第 4 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 7 条(其中 7 条首次提出)。发现反复回到同一批文件:scripts/tests/ci-platform-lanes.test.js(第 1、2、3 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
| [/needs\.classify_pr\.outputs\.skip_ci/g, '"false"'], | ||
| [/needs\.test\.outputs\.ci_profile/g, '"full"'], |
There was a problem hiding this comment.
[Suggestion] R2-1: (fix-induced) The round-3 fix for R2-1 replaced the connective-blind token probes with the substitute-then-evaluate helper gateRunsOn, but its needs-output substitutions pin each modeled output to a single value, and a gate keyed on any other value of a modeled output is silently evaluated at the pin. No current gate exhibits this — all eight use the pinned outputs only in their != 'true' / == 'full' shapes — so this is a latent hole in the guard, not a present bug. Concretely: a future job shaped like if: "!cancelled() && needs.test.outputs.ci_profile != 'full'" evaluates at the pin ("full" !== 'full' → false), assertEventReachesOnly('schedule', LANES) concludes it does not run on the nightly and passes, but in a real nightly test is excluded from schedule so its output is empty and !cancelled() overrides the skipped needs edge — '' != 'full' → true, and the job rides the nightly anyway while every test stays green: exactly the silent lane-widening this file exists to catch. Verified by probe: appending such a job keeps 24/24 green, while pointing the same job at an unmodeled output makes the guard throw — the documented fail-loud protection covers new terms only, not new comparisons on modeled terms. Document the limitation where the pins are declared:
| [/needs\.classify_pr\.outputs\.skip_ci/g, '"false"'], | |
| [/needs\.test\.outputs\.ci_profile/g, '"full"'], | |
| // These pins model a normal run: a gate keyed on any other value of a | |
| // modeled output (e.g. `ci_profile != 'full'`, `skip_ci == 'true'`) | |
| // evaluates here at the pin and may mis-report its reachability — | |
| // re-derive this helper's verdict before adding such a gate. | |
| [/needs\.classify_pr\.outputs\.skip_ci/g, '"false"'], | |
| [/needs\.test\.outputs\.ci_profile/g, '"full"'], |
中文说明
针对 R2-1 的第 3 轮修复用「替换后求值」的 gateRunsOn 助手替换了连接词盲的 token 探针,但其 needs 输出替换把每个建模输出钉死在单一取值上;任何以这些输出的其他取值为条件的 gate 都会按钉住的取值被静默求值。当前没有 gate 触发这一点——现有 8 个 gate 只以 != 'true' / == 'full' 的形态使用这些输出——因此这是守卫的潜在漏洞而非现实 bug。具体地:未来若新增形如 if: "!cancelled() && needs.test.outputs.ci_profile != 'full'" 的 job,按钉住值求值会得到 "full" !== 'full' → false,assertEventReachesOnly('schedule', LANES) 判定它不在 nightly 运行而通过;但真实 nightly 中 test 因 schedule 被排除而跳过,其输出为空,且 !cancelled() 会覆盖被跳过的 needs 边——'' != 'full' → true,该 job 照样搭上 nightly,而所有测试仍是绿的:这正是本文件要防的「静默扩大」。探针实测:追加这样一个 job 后 24/24 仍全绿;把同一 job 指向未建模的输出则守卫会抛错——文档中「未建模即抛错」的保护只覆盖新词项,不覆盖对已建模词项的新比较。建议在钉住值声明处补充注释说明该局限(见上方 suggestion 块)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // `!= 'push'`, so it passes them, is skipped by the exclusivity helper | ||
| // (test is in PUSH_JOBS), and leaves the routing tests green because they |
There was a problem hiding this comment.
[Suggestion] R4-1: This paragraph justifies the whole-literal pin below by claiming an allowlist-form rewrite of test.if without push "is skipped by the exclusivity helper (test is in PUSH_JOBS)" — but that was the round-3 helper's behavior, which this diff deleted. The current assertEventReachesOnly asserts both directions: allowed jobs MUST reach (reached === true), as its own doc comment says. Verified by probe: rewriting test.if into allowlist form without push fails the suite with "test no longer runs on push" — caught, not skipped. The paragraph's conclusion is therefore argued from a false premise, and a future editor who mutation-checks it will discount the whole paragraph — including the pin's real, still-valid justification: the evaluator substitutes !cancelled() to the constant true, so dropping !cancelled() from the gate leaves every evaluation-based assertion green and only this literal pin goes red. Rewrite the paragraph to match the current shape:
// contains no `!= 'push'`, so it passes the exclusion checks — but the
// exclusivity helper's positive half catches it (test is in PUSH_JOBS, so
// it must reach). The literal pin below covers what the evaluator cannot:
// `!cancelled()` is substituted to a constant, so a gate that loses it
// evaluates identically and only the whole-literal pin catches the change.
中文说明
这段注释为下方的整字面量钉住提供理由,声称「不含 push 的 allowlist 形式改写会被排他性助手跳过(test 在 PUSH_JOBS 中)」——但那是第 3 轮助手的写法,本 diff 已将其删除。当前的 assertEventReachesOnly 双向断言:允许的 job 必须到达(reached === true),其自身文档注释也如此说明。探针实测:把 test.if 改写为不含 push 的 allowlist 形式,套件会以 "test no longer runs on push" 失败——被抓住了,而非被跳过。因此该段结论建立在错误前提上;未来做变异核查的编辑会发现它与事实不符,进而连带怀疑整段注释——包括钉住真正仍然成立的理由:求值器把 !cancelled() 替换为常量 true,去掉 !cancelled() 的 gate 求值结果不变,所有基于求值的断言都保持绿色,只有这个整字面量钉住会变红。建议把该段改写为与现状一致的表述(见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
…fast-signal Resolve the .size-baseline conflict at the merged ci.yml's size (95501): main's 91370 entry is 4136 bytes under the merged file, past the 4096 allowance. Update the push-lane comments for the no-AK gate's move into its own integration_no_ak job (#10384), and address the round-4 review notes on ci-platform-lanes.test.js: document the modeled-output pin limitation in gateRunsOn, and rewrite the stale justification under the whole-literal test.if pin to match the evaluator-based helper.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #10345 (round 5)This round merged Conflict resolution (
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
scripts/tests/ci-platform-lanes.test.js:86 — [probe] case-insensitive-equality hole in the gateRunsOn exclusivity guard
中文说明
无阻断问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/main-ci-failure-issue.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
What this PR does
ci.ymlaccepts apushtomainagain, and thetestjob is the only job that takes it. Every other job keeps the gate it already had, so a post-merge run resolves to exactly two jobs — Classify PR and Test — and a nightly still resolves to exactly the two platform lanes. The expensive, PR-shaped steps insidetestalready carry their ownpull_request/merge_groupgates (the no-AK integration gate, the checkout-head verifier, the test reporter), so they skip themselves on a push and the lane comes out as lint, static analysis and unit tests. The lane is routed to the ECS pool throughpick_runner;classify_pr's ownruns-ondeliberately keeps no push arm, and a new routing test pins that asymmetry so a later "drift fix" has to read the reason first.Why it's needed
The
pushtrigger was removed on the premise, stated in the comment it left behind, that "the merge queue validates the merged tree before it lands, so there is nothing left to run on the post-merge push". No merge queue is enabled on this repository: there has been no queue run since 2026-07-02, andmain's ruleset carries nomerge_queuerule. The premise never held — nothing validated the merged tree before it landed, and after it landed the only remaining signal was the ~40-minute E2E workflow. That is how a regression can sit inmainfor hours before anything reports it.This PR does not gate merges and does not change whether anything can be merged; making a check required is a separate change. It restores the fast post-merge signal that the removal assumed something else was providing.
main-ci-failure-issue.ymlalready watchesQwen Code CIforworkflow_run.event == 'push'onmain, so this lane files an issue on failure with no further wiring.Reviewer Test Plan
How to verify
Run the two suites that pin the wiring this PR touches — the routing guards (which now cover the push arm and the kill-switch on it) and the platform-lane gate, whose "keeps a nightly run to exactly the two lanes" assertion is what proves the new trigger did not leak into the nightly:
Then confirm the trigger reaches exactly the intended jobs, by evaluating every job's
ifagainst every event on both trees. Substituting the context terms and evaluating the remaining&&/||skeleton is the same techniqueci-runner-routing.test.mjsalready uses, and any unmodelled context term fails loud rather than silently reading as false:The expected difference is two cells and nothing else:
classify_prandtestflip from skipped to running in thepushcolumn. Every other job on every other event is unchanged — in particular theschedulecolumn stays at exactlytest_macos+test_windows.Evidence (Before & After)
Job-versus-event matrix,
origin/mainon the left of the flip and this branch on the right:Step-level resolution of
teston a push — the 7 skipped steps are exactly the PR-shaped and profile-branch ones, and the no-AK integration gate is among them:That the no-AK gate skips matters for a second reason: the two suites currently failing on
main(integration-tests/sdk-typescript/permission-control.test.tsandtool-control.test.ts) live underintegration-tests/, which is not an npm workspace, andtest:ciisnpm run test:ci --workspaces … && npm run test:scripts. The standingmainbreakage is therefore outside this lane's reach and will not make it red on day one.Suites run locally, all green:
Tested on
The change is workflow YAML plus its guards, so it is platform-independent; the suites above were run locally on macOS, and Linux is covered by CI on this PR.
Environment (optional)
N/A — workflow and test changes only.
Risk & Scope
concurrencyalready collapsesmainto one pending run per group, so bursts coalesce rather than queue up.Linked Issues
None.
中文说明
这个 PR 做了什么
ci.yml重新接受推送到main的push事件,并且只有test这一个 job 接收它。其他每个 job 都保持原有的准入条件,因此一次合并后的运行恰好解析为两个 job —— Classify PR 和 Test —— 而 nightly 仍然恰好解析为两条平台 lane。test内部那些昂贵的、PR 形态的步骤本身就带有各自的pull_request/merge_group门(no-AK 集成门、checkout head 校验步骤、测试报告发布步骤),所以它们在 push 上会自行跳过,这条 lane 最终呈现为 lint、静态分析和单元测试。该 lane 通过pick_runner路由到 ECS 池;classify_pr自身的runs-on有意不加 push 分支,并且新增了一个路由测试把这个不对称钉住,好让日后想「修复 drift」的人必须先读到原因。为什么需要它
push触发器当初被移除,依据是它留下的注释里那句话:「合并队列会在合并树落地前验证它,所以合并后的 push 已经没有什么需要跑的了」。但本仓库并未启用合并队列:自 2026-07-02 以来没有任何队列运行,且main的 ruleset 中没有merge_queue规则。因此这个前提从未成立 —— 合并树在落地前没有被任何东西验证,落地之后唯一剩下的信号是耗时约 40 分钟的 E2E workflow。这正是一个回归能在main里静静躺上数小时才被报出来的原因。本 PR 不做合并门禁,也不改变任何东西能否被合并;把某个检查设为 required 是另一个独立的改动。它只是恢复了那个「移除时假定由别处提供」的快速合并后信号。
main-ci-failure-issue.yml已经在监听Qwen Code CI在main上workflow_run.event == 'push'的情形,所以这条 lane 失败时会自动开 issue,无需额外接线。评审验证方案
如何验证
先跑两个钉住本 PR 所改接线的测试套件 —— 路由守卫(现已覆盖 push 分支及其上的 kill-switch)和平台 lane 门禁,后者的「keeps a nightly run to exactly the two lanes」断言正是证明新触发器没有泄漏进 nightly 的依据:
然后确认这个触发器恰好只到达预期的 job:在两棵树上,对每个 job 的
if逐事件求值。把上下文项替换为字面量、再对剩下的&&/||骨架求值,这与ci-runner-routing.test.mjs已经在用的手法相同;任何未建模的上下文项都会显式报错,而不会被静默当作 false。求值脚本见上方英文正文中的代码块。预期差异只有两格,别无其他:
classify_pr和test在push列从跳过变为运行。其他所有 job 在其他所有事件上都保持不变 —— 特别是schedule列仍然恰好是test_macos+test_windows。证据(前后对比)
job × 事件矩阵见上方英文正文:
origin/main为变更前,本分支为变更后。test在 push 上的步骤级解析同样见上方英文正文:被跳过的 7 个步骤恰好是 PR 形态的步骤和 profile 分支步骤,其中包含 no-AK 集成门。no-AK 门被跳过还有第二重意义:当前在
main上失败的两个套件(integration-tests/sdk-typescript/permission-control.test.ts与tool-control.test.ts)位于integration-tests/之下,而该目录不是 npm workspace,且test:ci的定义是npm run test:ci --workspaces … && npm run test:scripts。因此main上这个既有的红不在本 lane 的射程之内,不会让它一上线就变红。本地运行的套件全部通过,结果见上方英文正文中的清单。
测试平台
本改动是 workflow YAML 及其守卫,与运行平台无关;上述套件在本地 macOS 上运行,Linux 由本 PR 的 CI 覆盖。
运行环境(可选)
N/A —— 仅涉及 workflow 与测试改动。
风险与范围
concurrency已经把main收敛为每组一个待运行的 run,所以密集合并会自行合并而不是排队堆积。关联 Issue
无。