ci: take the macOS and Windows lanes off pull requests - #10059
Merged
Conversation
Both lanes were costing contributors more than they were telling them.
Measured over the 18 hours before this change, on pull requests:
- Windows reported 13 failures and 0 successes. The failures are one
standing set of Windows-only path and symlink cases — resolved paths
into read_many_files, releaseWorktree through an ancestor symlink,
the SHA-256 review worktree — repeating across unrelated PRs, so the
red X almost never belonged to the diff under it.
- macOS queued for a p90 of 42 minutes and up to 159, on a hosted pool
this repository does not saturate by itself: 20 of the 63 sampled
waits happened with zero macOS jobs of ours running.
Neither lane gates a merge — the `main` ruleset carries no required
status check — so none of that waiting or noise was buying protection.
Leave them on the nightly, the merge queue and dispatch, and drop the
pull-request arm plus the classifier job that existed only to feed it.
That makes the nightly load-bearing rather than a backstop: it is now
the only report of a non-Linux regression, so add a guard for the three
ways it could go quiet — the schedule disappearing, a lane no longer
accepting it, or a third job joining it and failing the run for reasons
that have nothing to do with either platform.
The classifier, its script mode and both test files are left in place so
restoring the pull-request trigger, once the Windows failures are fixed,
is a revert plus two `if` arms.
Collaborator
|
|
Collaborator
|
Gate pass — one of the better-evidenced CI PRs this gate has seen.
Moving on to code review. 🔍 中文说明门禁通过——这是本门禁见过的证据最扎实的 CI 类 PR 之一。
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent proposal first, then the diff: I would have done exactly this — pull the Verified at the head commit (API reads only — no PR code executed):
No blocking findings; no nits worth listing. Test evidence — the PR's own CI, via APIThe strongest signal is behavioural and already visible on this PR itself: both platform lanes report Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Nothing user-visible changes and there is no behavioural product claim to settle — the observable outcome is this PR's own check runs, so no sandboxed-lane trigger applies. Real-scenario testing: unattended CI run, nothing user-visible — N/A. 中文说明代码审查先看独立方案再看 diff：我自己会这么做——把 在 head commit 上核实（只走 API 读取——未执行任何 PR 代码）：
无阻塞项，也没有值得列的吹毛求疵。 测试证据——PR 自己的 CI，经 API 读取最强的信号是行为性的，而且在本 PR 上已经可见：两条平台 lane 在本 PR 上报告 没有用户可见变化，也没有需要沙盒验证的行为性产品论断——可观测结果就是本 PR 自己的 check runs，因此不适用沙盒 lane 触发。真实场景测试：无人值守 CI 运行、无用户可见面——N/A。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — every factual claim in this PR checked out against the API, the diff is the minimal shape of the change, and the coverage tradeoff is argued with data and guarded with a test. Reflection, honestly: my independent proposal was this diff minus the guard test and the restoration runbook — the PR exceeds it. The one reservation any reviewer should hold is the timing shift: a macOS/Windows regression now lands on Approval is deferred: the PR's own CI (the Linux lane running the new guard, plus the Desktop Shell lanes) is still in flight on the reviewed commit. The approve-on-green marker below lets the finalize job land the commit-pinned approval once everything is green; if anything lands red, it will flag instead. 中文说明置信度：5/5 —— PR 里的每一条事实论断都经 API 核实，diff 是该改动的最小形态，覆盖面的代价有数据支撑、有守卫测试兜底。 诚实地复盘：我的独立方案就是这份 diff 去掉守卫测试和恢复手册——PR 超出了它。审查者唯一该保留的疑虑是时间差：macOS/Windows 回归今后会先落进 批准暂缓：PR 自己的 CI（跑新守卫的 Linux lane 和两条 Desktop Shell lane）在被审 commit 上仍在进行。下方的 approve-on-green 标记让 finalize 任务在全绿后落下绑定 commit 的批准；若有红色落地，它会改为标记异常。 — Qwen Code · qwen3.8-max Reviewed at |
yiliang114
previously approved these changes
Aug 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Pull-request trigger removed from test_macos/test_windows with data behind it (Windows lane 13 failure / 0 success over 18h from standing path/symlink failures; macOS p90 queue wait 42m; neither lane is a required check). The classify_platform job removal is clean and revert-ready (classifier files left intact, restore path documented), the new platform-lane-triggers.test.mjs pins the nightly schedule as the lanes' remaining signal (schedule exists, both lanes accept it, and a nightly is exactly those two jobs), and it is wired into HELPER_TESTS. Two checks still pending at review time (review-pr, Test ubuntu); no failures observed.
Contributor
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. |
The lane change left scripts/tests/ci-platform-lanes.test.js pinning the shape it removed — a sensitive-PR trigger arm and a live classify_platform job — which failed the Test job on this branch. Rewrite the suite to pin the new contract instead: both lanes run on the schedule, the queue and dispatch and on nothing else; the pull-request arm and the classifier are gone whole (a half-restoration — a trigger without its classifier, or the reverse — fails); the classifier's own scripts stay tested so restoring the trigger stays a clean revert; the nightly still reaches exactly the two lanes and its failure still files an issue. That suite already owned the nightly-liveness assertions, so the platform-lane-triggers.test.mjs guard added earlier on this branch duplicated it — dropped, along with its HELPER_TESTS entry. Verified by mutation: deleting the schedule, dropping schedule from one lane, letting a third job onto the nightly, and reintroducing the pull-request arm without its classifier each fail the suite; the branch shape passes 17/17.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 5": live execution of scripts/tests/ci-platform-lanes.test.js — neither the worktree nor the parent checkout has node_modules , and a full monorepo install excee….
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.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 5"：live execution of scripts/tests/ci-platform-lanes.test.js — neither the worktree nor the parent checkout has node_modules , and a full monorepo install excee…。
未检查（工具限制，非阻断）：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.0)
Comment on lines
+59
to
+60
| expect(cond).toMatch(/event_name == 'schedule'\s*\|\|/); | ||
| expect(cond).toContain("github.event_name == 'workflow_dispatch'"); |
Collaborator
There was a problem hiding this comment.
[Suggestion] R1-1: Every other comment in both files was rewritten to the new three-trigger contract, except one: the rationale comment of the ${lane}'s steps are gated for every trigger it now has test (line 121, just below this hunk) still reads "for every step in a job that now runs on four". With the pull_request arm removed, the lanes run on three triggers — merge_group, schedule, workflow_dispatch — exactly what the assertions here pin. A maintainer auditing which step gates are still needed, or preparing the restoration this file's header describes, reads "four" beside the very test that protects that invariant and reasons from a trigger list that includes an event the job no longer accepts. Change "runs on four" to "runs on three".
中文说明
两个文件里的其他注释都已改写为新的三触发器契约，唯独一处例外：${lane}'s steps are gated for every trigger it now has 测试（就在本代码块下方第 121 行）的理由注释仍写着 "for every step in a job that now runs on four"。pull_request 分支移除后，这两条 lane 只在三个触发器上运行——merge_group、schedule、workflow_dispatch——正是这里断言所钉住的契约。维护者审计哪些 step 门控仍然需要、或准备本文件头部描述的恢复操作时，会在这个守护该不变量的测试旁读到 "four"，从而基于一个包含 job 已不再接受的事件的触发器列表来推理。请把 "runs on four" 改为 "runs on three"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
wenshao
enabled auto-merge
August 26, 2026 03:29
qqqys
approved these changes
Aug 26, 2026
yiliang114
approved these changes
Aug 26, 2026
This was referenced Aug 26, 2026
This was referenced Aug 26, 2026
Merged
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes the
Test (macos-latest, …)andTest (windows-latest, …)lanes off the pull-request trigger. They keep running on the nightly schedule, in the merge queue, and on dispatch.Why
Measured across every macOS/Windows job in
ci.yml,e2e.yml,sdk-java.ymlandlive-host.ymlover the 18 hours before this change:Windows is not slow, it is red. The 13 failures are one standing set of Windows-only path and symlink cases repeating across unrelated PRs —
Session.test.ts > passes resolved paths to read_many_files tool,git.integration.test.ts > releaseWorktree > refuses to release through an ANCESTOR symlink,agent-prompt.test.ts > pins a SHA-256 review worktree. Sampled four failing jobs on four different PRs; same signatures each time. A contributor reading that red X is reading something no diff of theirs caused.macOS is not ours to unblock. Peak concurrency from this repository was 7, average 1.62, and 20 of the 63 sampled waits happened with zero macOS jobs of ours running — that is the hosted pool, not self-inflicted congestion. Shedding our own usage shortens the queue but cannot fix it, and
ci.ymlis only 31% of it (8.9 of 29.1 macOS runner-hours;e2e.yml9.6h,live-host.yml7.6h,sdk-java.yml2.0h).Neither lane gates a merge. Ruleset
9234124onmaincarries onlydeletion,non_fast_forwardandpull_request— no required status check — and the merge queue has not run since 2026-07-02. So every one of those waits and red Xs was paid for and bought nothing.What changes
test_macos/test_windows: thepull_requestarm is dropped from bothifgates.merge_group,scheduleandworkflow_dispatchare untouched.classify_platformis removed. It existed only to answer "does this PR need the lanes", and with the lanes off the pull-request path nothing consumed its output. Its classifier (classify-platform-sensitivity.mjs), theplatformmode ofclassify-pr-profile.sh, and both test files are deliberately left in place.scripts/tests/ci-platform-lanes.test.js— the existing suite pinning these lanes' wiring — is rewritten to the new contract (the first CI round on this branch failed precisely because it still pinned the old one): both lanes run on the schedule, the queue and dispatch and on nothing else; the pull-request arm and the classifier are gone whole, and a half-restoration (a trigger without its classifier, or the reverse) fails it; the classifier's own scripts stay tested so restoring the trigger stays a clean revert; the nightly still reaches exactly the two lanes, and a failing scheduled run onmainstill files an issue viamain-ci-failure-issue.yml— the nightly is now the only report of a non-Linux regression (the shape fix(ci): self-heal failed checkouts on the reused review runners #9220 shipped from), but it is not a silent one.Desktop Shell (windows-2022)is not touched: 30/30 green, ~31s, with its own path short-circuit. It costs nobody anything.Restoring this
Fixing the Windows failures is the actual work; restoring the trigger is its last step, not a separate decision. It is a revert of this commit plus putting the
pull_requestarm back on both lanes. Worth reconsidering at that point: the classifier calls every fork PR platform-sensitive (IS_SAME_REPO_PR != 'true'→sensitive=true), which is most of the traffic here and most of what the macOS queue was spent on — even though the classifier is checked out frombase.shaand only reads the changed-file list, so it is not obvious it needs to be that conservative.Verification
actionlint(1.7.12) andyamllintclean;shellcheckunchanged; prettier clean.ci-runner-routing.test.mjsand both classifier suites.ifagainst each event, base vs this branch:pull_request8 jobs → 5 with no platform lanes;schedule2 jobs,merge_group6,workflow_dispatch6, all unchanged.schedulefrom a lane, letting a third job onto the nightly, and reintroducing the pull-request arm without its classifier. Each one fails it; the branch shape passes 17/17.中文说明
把
Test (macos-latest, …)和Test (windows-latest, …)两条 lane 从 pull request 触发中摘掉。nightly、merge queue 和手动 dispatch 照常运行。为什么
统计改动前 18 小时内
ci.yml、e2e.yml、sdk-java.yml、live-host.yml的全部 macOS/Windows job：Windows 不是慢，是红。 那 13 次失败是同一批 Windows-only 的路径/符号链接用例在互不相关的 PR 上反复出现——
Session.test.ts > passes resolved paths to read_many_files tool、git.integration.test.ts > releaseWorktree > refuses to release through an ANCESTOR symlink、agent-prompt.test.ts > pins a SHA-256 review worktree。抽查了 4 个不同 PR 上的失败 job，签名一致。贡献者看到的这个红叉，跟他自己的 diff 没有关系。macOS 的堵不是我们能解开的。 本仓库的 macOS 峰值并发只有 7、平均 1.62，而 63 个采样等待里有 20 个发生在我们自己一个 macOS job 都没跑的时候——那是托管池本身在排队，不是自己挤自己。削减自己的用量能缩短队列但治不了它，何况
ci.yml只占其中 31%（29.1 macOS runner-hours 里的 8.9h；e2e.yml9.6h、live-host.yml7.6h、sdk-java.yml2.0h）。两条 lane 都不拦合并。
main上的 ruleset9234124只有deletion、non_fast_forward、pull_request三条规则，没有任何 required status check，而 merge queue 自 2026-07-02 起就没跑过。所以这些等待和噪声全是净支出。改了什么
test_macos/test_windows：两个if门里的pull_request分支删掉，merge_group、schedule、workflow_dispatch原样保留。classify_platform。它存在的唯一目的是回答"这个 PR 需不需要跑这两条 lane"，lane 不再走 PR 路径后就没有消费者了。它的分类器classify-platform-sensitivity.mjs、classify-pr-profile.sh的platform模式、以及两份测试都刻意保留原样。scripts/tests/ci-platform-lanes.test.js到新契约（本分支第一轮 CI 失败正是因为它还钉着旧契约）：两条 lane 只在 schedule/merge queue/dispatch 上跑；pull-request 分支和分类器整体移除，半恢复（有触发没分类器，或反之）会被测出来；分类器脚本自身保持有测试，恢复时仍是干净的 revert；nightly 仍然恰好只有这两个 job，且main上的 nightly 失败仍会经main-ci-failure-issue.yml自动开 issue——nightly 现在是非 Linux 回归的唯一报告渠道（fix(ci): self-heal failed checkouts on the reused review runners #9220 出事的形态），但它不是哑的。Desktop Shell (windows-2022)没动：30/30 全绿、约 31 秒、自带 path 短路，不构成任何负担。怎么加回来
修掉 Windows 那批失败才是正事，恢复触发是它的最后一步，不是一个独立决定。做法是 revert 本 commit，再把
pull_request分支加回两条 lane。到那时值得一并重新考虑的是：分类器把每一个 fork PR 都判为 platform-sensitive（IS_SAME_REPO_PR != 'true'→sensitive=true），而 fork PR 是这里的主要流量、也是 macOS 队列的主要去处——但分类器本身是从base.sha检出、只读 changed-file 列表的，未必需要这么保守。验证
actionlint（1.7.12）与yamllint干净；shellcheck无变化；prettier 干净。ci-runner-routing.test.mjs和两个分类器套件。if，base 与本分支对比：pull_request从 8 个 job 降到 5 个且不含两条 lane；schedule2 个、merge_group6 个、workflow_dispatch6 个均无变化。