Skip to content

ci: stop failing the Test job on a starved vitest worker RPC alone - #11103

Open
yiliang114 wants to merge 3 commits into
QwenLM:mainfrom
yiliang114:ci/ecs-on-task-update-flake
Open

ci: stop failing the Test job on a starved vitest worker RPC alone#11103
yiliang114 wants to merge 3 commits into
QwenLM:mainfrom
yiliang114:ci/ecs-on-task-update-flake

Conversation

@yiliang114

@yiliang114 yiliang114 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Stops the Linux Test job from going red when the only thing that broke is vitest's own worker IPC. It adds .github/scripts/ci/classify-infra-flake.mjs, which the test step consults after a non-zero npm run test:ci:workspaces, and wires it in with a ::warning:: on a tolerated run plus a QWEN_CI_TOLERATE_RPC_TIMEOUT kill switch.

Why it's needed

Vitest reports a worker whose onTaskUpdate RPC went unanswered as a run-level unhandled error and exits 1 for it even when every test passed. That RPC is worker→collector IPC with a fixed 60s budget (#10438), so it starves on CPU contention rather than on elapsed time.

On the shared ecs-qwen pool — measured at load 218–270 with 37–74 concurrent vitest processes per host — five of thirteen consecutive red Test jobs on 2026-09-05 had zero failing tests. cli printed Test Files 1014 passed (1014) / Tests 28582 passed | 90 skipped and core printed 643 passed | 1 skipped / 23521 passed, then each exited 1 on one to three of these timeouts. Sampled runs: 33953519066, 33952382944, 33952315316, 33948811958, 33948268415.

Nothing already in the lane reaches it:

  • --retry=2 retries individual tests; this is reported after the run.
  • testTimeout / hookTimeout (60s on ECS) bound a test, not the collector channel.
  • The 110-minute step ceiling never engages — no job in that sample was cancelled, and the longest test step finished on its own at 96m55s.

Why not just set dangerouslyIgnoreUnhandledErrors on Linux. Every vitest config already carries it as process.platform !== 'linux', pinned by scripts/tests/unit-vitest-configs.test.ts. Linux is deliberately left fatal, and rightly: packages/core/src/extension/archive-safety.test.ts documents a minipass write after end that escaped as an uncaught exception under contention and exited an all-green suite non-zero (release run 33576013293). That was a real defect in an unawaited stream, and the remedy was to fix the test — flipping the flag would blind the lane to the next one.

That is the distinction this draws. onTaskUpdate is vitest's own IPC and carries no information about the code under test; an escaping product exception does. A whole-run boolean cannot tell them apart, so the tolerance is by signature instead.

Reviewer Test Plan

How to verify

The classifier fails closed on every branch. It tolerates only when all of these hold:

  1. the log carries [vitest-worker]: Timeout calling "onTaskUpdate";
  2. vitest's own tally (Vitest caught N unhandled error…, summed across projects) equals the signature count — so an unrelated unhandled error riding along in the same run is never masked by them;
  3. no real FAIL line appears anywhere (reuses the already-tested extractFailingTests from ci/main-failure-signature.mjs);
  4. npm named the failing workspaces via npm error path …/packages/<name>;
  5. each named workspace wrote a junit.xml whose <testsuites> totals report failures="0" — a worker that was OOM-killed or segfaulted writes no junit and stays red.

Condition 5 is load-bearing: vitest always writes tests/failures/errors/time and never increments errors, so run-level unhandled errors do not reach junit at all, which makes failures the authoritative "did a test fail" signal.

node --test --test-concurrency=1 .github/scripts/ci/classify-infra-flake.test.mjs
ℹ tests 16
ℹ pass 16
ℹ fail 0

The strongest evidence is a replay against the thirteen real job logs from that window — five tolerated, eight refused, matching a manual classification of the same runs:

run runner peak load real test failures verdict
33953519066 ecs-qwen-hk3-20 249.7 0 tolerate
33952382944 ecs-qwen-hk3-7 218.5 0 tolerate
33952315316 ecs-qwen-hk3-12 218.5 0 tolerate
33948811958 ecs-qwen-hk3-11 228.3 0 tolerate
33948268415 ecs-qwen-hk5-2 231.6 0 tolerate
33952837177 ecs-qwen-hk5-2 270.4 1 (MessageList.dom.test.tsx) refuse
33952335901 ecs-qwen-hk5-22 270.4 1 (MessageList.dom.test.tsx) refuse
33947916462 ecs-qwen-hk3-17 218.1 1 (MessageList.dom.test.tsx, on main) refuse
33950287375 ecs-qwen-hk3-19 218.5 1 (InProcessBackend.test.ts) refuse
33947875016 ecs-qwen-hk5-32 234.7 1 (BackgroundTasksDialog.test.tsx) refuse
33949121787 ecs-qwen-hk3-9 228.0 1 (build-artifact.test.ts) refuse
33952947190 GitHub-hosted 5.9 1 (acpAgent.test.ts) refuse
33947741747 GitHub-hosted 5.5 1 (no-ak-integration-ci.test.js) refuse

The two healthy hosted runners are the useful control: load ~5.5, zero RPC timeouts, and their failures were genuine — the classifier refuses both, on condition 1 rather than condition 3.

Guard suites, all green on the rebased branch:

node --test --test-concurrency=1 <24 of the 25 HELPER_TESTS>   → ℹ tests 426  ℹ pass 422  ℹ fail 0
npx vitest run --config ./scripts/tests/vitest.config.ts \
  ci-platform-lanes no-ak-integration-ci unit-vitest-configs package-scripts capture-tmux-ci
                                                                → Test Files 6 passed, Tests 130 passed | 2 skipped

qwen-triage-workflow.test.mjs is the 25th and is excluded above: it fails identically on a pristine origin/main checkout on this machine (45 failures, byte-identical failing set), because its shell-driven cases need bash 4+ and macOS ships 3.2. Same reason check-workflow-size.sh cannot run locally (declare -A); its arithmetic was checked by hand instead — ci.yml grows to 134204 bytes against a 131378 baseline, 2826 of the 4096 allowance.

bash -n on the extracted run: block passes, and the block's TMPDIR routing (lines 16–22) precedes the new WORKSPACES_LOG assignment (line 40), so the captured log lands in the disk-backed temp the EXIT trap already cleans. The cancelled run 33965479612 on this PR confirms the pipeline spawns as intended on a real ECS runner — its orphan cleanup names both npm run test:ci:workspaces --retry=2 and tee as live processes.

Evidence (Before & After)

N/A — no user-visible surface. A tolerated run is not silent: it emits one ::warning:: naming the runner, the peak DFSAMPLE load, the suppressed timeout count and #10879, so over-capacity stays visible instead of going quiet behind a green job.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux ⚠️

Only the Linux test leg calls the classifier; the macOS and Windows legs already get the exemption through dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux'. The classifier itself is platform-independent and was exercised on macOS against real Linux CI logs. Linux end-to-end is what this PR's own CI run will supply.

Environment (optional)

Unit tests and guard suites only — node --test and npx vitest run against a local npm ci checkout. No daemon, sandbox or browser involved.

Risk & Scope

  • Main risk or tradeoff: a real unhandled error could be tolerated if it exactly mimicked the signature. Three independent conditions bound that: the signature is matched literally, vitest's own unhandled tally must equal the signature count, and every blamed workspace must produce a junit with failures="0". The residual risk is a product defect that manifests only as a worker RPC stall with nothing else wrong — which by definition leaves no other evidence to act on. QWEN_CI_TOLERATE_RPC_TIMEOUT=0 reverses the whole behaviour without a PR.
  • Not validated / out of scope: the tolerance can only engage for packages/cli, packages/core and packages/web-shell. Of the 27 workspaces, 22 carry a test:ci script and so run under npm run test:ci:workspaces, but only those 3 add a junit reporter — condition 5 has nothing to read for the other 19, so they refuse with wrote no junit.xml and the job stays red. That is fail-closed and therefore safe, and it does cover the three legs the thirteen-run sample implicates, but the classifier is narrower than test:ci:workspaces looks. Extending junit reporters to the remaining 19 configs touches the unit-vitest-configs.test.ts pin and is deliberately left to a follow-up. Also out of scope: the pool capacity itself (hk4 is pinned as the release host but still carries the shared ecs-qwen label, so releases compete with PR CI #10879), which this treats honestly rather than fixes.
  • Breaking changes / migration notes: none. No public API, no config schema, no behaviour change on a healthy runner — the classifier is only consulted after a non-zero exit.

Linked Issues

Non-closing references; this PR resolves none of them.

中文说明

这个 PR 做了什么

当唯一出问题的东西是 vitest 自己的 worker IPC 时,不再让 Linux 的 Test job 变红。新增 .github/scripts/ci/classify-infra-flake.mjs,由 test 步骤在 npm run test:ci:workspaces 返回非零之后调用;接入时会在放行一次运行的同时发出 ::warning::,并提供 QWEN_CI_TOLERATE_RPC_TIMEOUT 作为应急开关。

为什么需要

vitest 会把「某个 worker 的 onTaskUpdate RPC 没有应答」记为 run 级的 unhandled error,并据此以退出码 1 结束——即使所有测试都通过。这个 RPC 是 worker→collector 的进程间通信,预算固定为 60 秒(#10438),所以它是因为 CPU 争抢而饿死,不是因为耗时超限。

在共享的 ecs-qwen 池上——实测 load 218–270、同主机并发 37–74 个 vitest 进程——2026-09-05 连续 13 个变红的 Test job 里,有 5 个的失败测试数为零。cli 打印的是 Test Files 1014 passed (1014) / Tests 28582 passed | 90 skipped,core 打印的是 643 passed | 1 skipped / 23521 passed,然后各自因为 1 到 3 个这类超时以退出码 1 结束。抽样运行:33953519066、33952382944、33952315316、33948811958、33948268415。

现有这条 lane 里没有任何机制能够覆盖它:

  • --retry=2 重试的是单个测试;而这个问题是在整轮跑完之后才上报的。
  • testTimeout / hookTimeout(ECS 上 60 秒)约束的是单个测试,不是 collector 通道。
  • 110 分钟的步骤上限根本不会触发——那批样本里没有任何 job 是 cancelled,最长的 test 步骤自己在 96 分 55 秒时结束。

为什么不直接在 Linux 上打开 dangerouslyIgnoreUnhandledErrors 每个 vitest 配置都已经带着它,取值是 process.platform !== 'linux',并由 scripts/tests/unit-vitest-configs.test.ts 钉住。Linux 被刻意保留为 fatal,而且这个决定是对的:packages/core/src/extension/archive-safety.test.ts 记录过一个 minipass 的 write after end,它在争抢下作为 uncaught exception 逃逸,把一个全绿的套件判成非零退出(release run 33576013293)。那是一个未被 await 的流里的真实缺陷,正确的处理是修测试——翻这个 flag 会让这条 lane 对下一个同类问题失明。

这正是本 PR 要划清的区别。onTaskUpdate 是 vitest 自己的 IPC,不携带任何关于被测代码的信息;而逃逸出来的产品异常携带。一个作用于整轮运行的布尔值无法区分两者,所以这里的容忍是按签名进行的。

评审测试计划

如何验证

分类器在每一个分支上都 fail closed。只有当以下条件全部成立时才放行:

  1. 日志里出现 [vitest-worker]: Timeout calling "onTaskUpdate"
  2. vitest 自己给出的计数(Vitest caught N unhandled error…,跨 project 求和)恰好等于签名出现次数——因此同一轮里夹带的其他种类 unhandled error 绝不会被这些超时掩护过去;
  3. 日志中任何位置都没有真实的 FAIL 行(复用 ci/main-failure-signature.mjs 里已有测试覆盖的 extractFailingTests);
  4. npm 通过 npm error path …/packages/<name> 指名了失败的 workspace;
  5. 每个被指名的 workspace 都写出了 junit.xml,且其 <testsuites> 汇总为 failures="0"——被 OOM kill 或 segfault 的 worker 写不出 junit,因此保持红。

条件 5 是承重的:vitest 总会写出 tests/failures/errors/time,且从不递增 errors,所以 run 级的 unhandled error 根本不会进入 junit——这使 failures 成为「有没有测试真的失败」的权威信号。

单测结果为 16 tests / 16 pass / 0 fail。

最有力的证据是把那个时间窗内 13 份真实 job 日志回放一遍——放行 5 个、拒绝 8 个,与对同一批运行的人工分类完全一致(详见上方表格)。其中 MessageList.dom.test.tsx 在 3 个运行里失败(含 main 本身),InProcessBackend.test.tsBackgroundTasksDialog.test.tsxbuild-artifact.test.ts 各 1 个,另有 2 个落在健康的 GitHub 托管 runner 上、失败是真实的(acpAgent.test.tsno-ak-integration-ci.test.js)。

那两个健康的托管 runner 是有用的对照组:load 约 5.5、零 RPC 超时,而它们的失败是真实的——分类器把两个都拒绝了,依据是条件 1 而不是条件 3。

守卫套件在 rebase 后的分支上全绿:24 个 HELPER_TESTS 文件跑出 426 tests / 422 pass / 0 fail;vitest 侧的 ci-platform-lanes、no-ak-integration-ci、unit-vitest-configs、package-scripts、capture-tmux-ci 共 6 个文件 130 passed / 2 skipped。

第 25 个 qwen-triage-workflow.test.mjs 被排除在上面之外:它在本机对纯净 origin/main 检出也以完全相同的方式失败(45 个失败,失败集合逐字节相同),原因是它的 shell 驱动用例需要 bash 4+ 而 macOS 自带 3.2。check-workflow-size.sh 因同样原因无法在本机运行(declare -A),所以改为手工核算——ci.yml 增长到 134204 字节,基线 131378,用了 4096 额度中的 2826。

对抽出的 run: 块执行 bash -n 通过;该块的 TMPDIR 路由(16–22 行)在新增的 WORKSPACES_LOG 赋值(40 行)之前,所以捕获的日志落在 EXIT trap 已经会清理的磁盘型临时目录里。本 PR 那次被取消的运行 33965479612 反而证实了管道在真实 ECS runner 上按预期启动——它的孤儿进程清理同时点名了 npm run test:ci:workspaces --retry=2tee 两个存活进程。

前后对比证据

不适用——没有用户可见界面。被放行的运行不是静默的:它会发出一条 ::warning::,写明 runner、DFSAMPLE 峰值负载、被抑制的超时次数以及 #10879,因此「池子超容量」这件事仍然可见,不会躲在一个绿色 job 后面安静下来。

测试平台

macOS ✅;Windows 不适用;Linux ⚠️ 未本地验证。

只有 Linux 的 test leg 会调用分类器;macOS 与 Windows 的 leg 已经通过 dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux' 获得豁免。分类器本身与平台无关,是在 macOS 上针对真实的 Linux CI 日志验证的。Linux 的端到端验证由本 PR 自己的 CI 运行提供。

环境(可选)

只涉及单测与守卫套件——在本地 npm ci 检出上跑 node --testnpx vitest run。不涉及 daemon、sandbox 或浏览器。

风险与范围

  • 主要风险或取舍: 如果一个真实的 unhandled error 精确地伪装成该签名,就有可能被放行。三个相互独立的条件限制了这种可能:签名是字面匹配的、vitest 自己的 unhandled 计数必须等于签名次数、每个被指名的 workspace 都必须产出 failures="0" 的 junit。剩下的残余风险是「某个产品缺陷只表现为 worker RPC 停滞、且没有任何其他异常」——而这种情况按定义就不会留下其他可据以行动的证据。QWEN_CI_TOLERATE_RPC_TIMEOUT=0 可以在不提 PR 的情况下整体回退这个行为。
  • 未验证 / 超出范围: 这个容忍只可能对 packages/clipackages/corepackages/web-shell 生效。27 个 workspace 中有 22 个带 test:ci 脚本、因此会被 npm run test:ci:workspaces 跑到,但只有这 3 个加了 junit reporter——条件 5 对另外 19 个无文件可读,于是它们以 wrote no junit.xml 被拒绝、job 保持红。这是 fail-closed 的,因此是安全的,也确实覆盖了 13 次运行样本所涉及的那三个 leg,但分类器的覆盖面比 test:ci:workspaces 看上去要窄。把 junit reporter 扩展到其余 19 个配置会动到 unit-vitest-configs.test.ts 的钉住,刻意留给后续 PR。同样超出范围:池子容量本身(hk4 is pinned as the release host but still carries the shared ecs-qwen label, so releases compete with PR CI #10879)——本 PR 诚实地处理症状,而不是修根因。
  • 破坏性变更 / 迁移说明: 无。不涉及公开 API、配置 schema,也不改变健康 runner 上的行为——分类器只在退出码非零之后才被调用。

关联 Issue

均为非关闭式引用;本 PR 不解决其中任何一个。

Vitest reports a worker whose onTaskUpdate RPC went unanswered as a run-level unhandled error and exits 1 for it even when every test passed. The RPC is worker->collector IPC with a fixed 60s budget (QwenLM#10438), so it starves on CPU contention rather than on elapsed time. On the shared ecs-qwen pool, measured at load 218-270 with 37-74 concurrent vitest processes per host, five of thirteen consecutive red Test jobs on 2026-09-05 had zero failing tests: cli printed '1014 passed (1014)' and core '643 passed | 1 skipped', then each exited 1 on one to three of these timeouts.

Nothing already in the lane covers it. --retry=2 is per test and this is reported after the run; testTimeout/hookTimeout bound a test, not the collector channel; and the 110-minute step ceiling never engages, since no job in that sample was cancelled and the longest test step finished on its own at 96m55s.

The sanctioned fix elsewhere here is dangerouslyIgnoreUnhandledErrors, which every vitest config carries as 'process.platform !== linux' and unit-vitest-configs.test.ts pins. Linux stays fatal deliberately: archive-safety.test.ts documents a minipass 'write after end' that escaped as an uncaught exception under contention and exited an all-green suite non-zero (release run 33576013293). That was a real defect in an unawaited stream, so flipping the flag on Linux would blind the lane to the next one. This draws the distinction that flag cannot: onTaskUpdate is vitest's own IPC and says nothing about the code under test, so tolerate it by name and keep everything else fatal.

Every branch fails closed. Tolerated only when the log carries the signature, vitest's own unhandled tally equals the signature count (so a second kind of unhandled error riding along is never masked), no real FAIL line appears anywhere, npm named the failing workspaces, and each of those wrote a junit.xml reporting zero failures - a worker that was OOM-killed or segfaulted writes no junit and stays red. A tolerated run emits a ::warning:: carrying the runner and the peak DFSAMPLE load, so over-capacity stays visible instead of going quiet behind a green job.

Replayed against the thirteen real job logs from that window: five tolerated, eight refused, matching a manual classification of the same runs. QWEN_CI_TOLERATE_RPC_TIMEOUT=0 restores the old behaviour without a PR.
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 0e97aa1 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 0e97aa1 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yiliang114 the description is missing three sections that .github/pull_request_template.md requires — and that the last six PRs merged into main all carry, five of them yours (#11095, #11078, #11075, #11049, #11037):

  • ## Risk & Scope — the three bullets (main risk or tradeoff / not validated or out of scope / breaking changes). The content exists, but it's folded into ## Notes.
  • ## Linked Issues#10438, #10892, #10879 and #10490 are referenced in prose under ## Notes. The section exists so those land in one predictable place. Keeping them non-closing is the right call here, since this PR closes none of them.
  • The <details><summary>中文说明</summary> block — the template asks for a full paragraph-by-paragraph translation of the English body, not a summary.

Template: https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md

This is a description-only ask. I've deliberately stopped short of reviewing the code so the two don't get tangled, and I'll pick the review up on the next pass once the sections are in.

One thing worth putting in ## Risk & Scope while you're there, because it is squarely a "not validated / out of scope" line: the tolerance can only ever engage for packages/cli, packages/core and packages/web-shell. Those are the only 3 of the 23 workspace vitest configs that add a junit reporter — and condition 5 needs a junit to read. The other 17 workspaces that run under npm run test:ci:workspaces write none, so they refuse with wrote no junit.xml and the job stays red. That's fail-closed and therefore safe, and it does cover the three legs your thirteen-run sample actually implicates. But a reader who assumes the classifier covers the whole test:ci:workspaces set will be surprised the first time a channels/* leg starves.

中文说明

@yiliang114 PR 描述缺少 .github/pull_request_template.md 要求的三个部分——而最近合并进 main 的六个 PR 都带有这三部分,其中五个是你自己的(#11095#11078#11075#11049#11037):

  • ## Risk & Scope —— 三个要点(主要风险或取舍 / 未验证或超出范围 / 破坏性变更)。内容其实已经写了,只是被并入了 ## Notes
  • ## Linked Issues —— #10438#10892#10879#10490 目前以正文形式写在 ## Notes 下面。设置这一节是为了让这些引用集中在固定位置。这里保持非关闭式引用是正确的,因为本 PR 不会关闭其中任何一个。
  • <details><summary>中文说明</summary> 区块 —— 模板要求对英文正文逐段完整翻译,而不是摘要。

模板地址:https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md

这只是对描述的要求。我特意没有继续审代码,以免两件事混在一起;等这些部分补齐后,下一轮我会接着审。

趁补 ## Risk & Scope 的时候,有一件事值得写进去,因为它正好属于"未验证 / 超出范围":这个容忍逻辑实际上只可能对 packages/clipackages/corepackages/web-shell 生效。在 23 个 workspace vitest 配置中,只有这 3 个加了 junit reporter——而条件 5 需要读 junit。npm run test:ci:workspaces 下运行的另外 17 个 workspace 完全不写 junit,因此会以 wrote no junit.xml 被拒绝,job 仍然变红。这是 fail-closed 的,因此是安全的,也确实覆盖了你 13 次运行样本中实际涉及的那三个 leg。但如果有人以为这个分类器覆盖了整个 test:ci:workspaces 集合,那么第一次遇到 channels/* leg 被饿死时会感到意外。

Qwen Code · qwen3.8-max-2026-09-02

@yiliang114

Copy link
Copy Markdown
Collaborator Author

All three sections are in, plus ### Environment.

On the junit coverage point — you were right and it is now the first line of "not validated / out of scope". I re-counted rather than take the figure on faith, and the shape is slightly worse than 3-of-23:

$ node -e '/* walk root package.json "workspaces", keep those with a test:ci script */'
workspace 总数: 27
有 test:ci 脚本(会被 test:ci:workspaces 跑到): 22
其中 vitest 配置带 junit: 3 → packages/web-shell, packages/core, packages/cli
不带 junit 的: 19

So the denominator that matters is 22, not 23 — 19 of the workspaces the classifier could be asked about can never satisfy condition 5 and refuse with wrote no junit.xml. Fail-closed, and it does cover the three legs the thirteen-run sample implicates, but "narrower than test:ci:workspaces looks" is the honest description and it is written that way now.

I left extending the junit reporter to the other 19 out of this PR on purpose: it touches 19 configs plus the unit-vitest-configs.test.ts pin, which is a different review than "stop calling a starved RPC a broken build". Happy to file it as a follow-up if you think the gap is worth closing before this tolerance earns any trust — the argument for doing it now is that a channels/* leg starving would look exactly like the five runs this PR exists for, and stay red.

Description-only change; no code touched since your pass.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 165 passed · 2 failed · 167 total

Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:165 通过 · 2 失败 · 167 总计

抖动门:✅ 1 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #11103 deep verification — ci: stop failing the Test job on a starved vitest worker RPC alone

Verdict: findings — 167 scripted assertions, 165 pass / 2 fail. The central claim is
proven load-bearing by an A/B on the verbatim CI shell (exactly one of seven cells flips,
red → green). The two failures are both in the PR's own new test suite, not in its behaviour:
two guards are load-bearing but unpinned. Neither is a merge blocker; both are one-test fixes.

Verified head: 0e97aa165c9a035962d64e603e89b2db39120d94 (git rev-parse HEAD^2)
Base: 82612e358422d53492ccd7f79b96db0b524e6397 (HEAD^1) — matches the snapshot's baseRefOid/headRefOid.
Single-commit PR; git rev-list HEAD^1..HEAD^2 returns 1 commit, equal to the snapshot's commits length, so per-commit attribution is reachable here despite the depth-2 checkout.
Environment: this CI container — Linux, node v22.23.2, bash 5.2.15, vitest 3.2.7, npm ci + npm run build already done at HEAD.
Live host load during this round: 256.69 / 252.51 / 246.87 — an independent sample of the same shared pool the PR describes at 218–270.

中文摘要

结论:findings —— 167 条脚本化断言,165 通过 / 2 失败。核心主张已由 A/B 证明是承重的:在逐字抽取的 CI shell 上跑七个 cell,只有一个 cell 的退出码发生变化(红 → 绿)。两条失败都出在 PR 自己新增的测试里,而不是它的行为里:有两个 guard 是承重的、但没有被测试钉住。两者都不是合并阻塞项,都是一条测试就能补上的。

A/B 结论(见下方 A/B table01-ab-run-block-base-vs-head.png):把 base 与 head 的 run: 块用 YAML 解析逐字抽出,在 Actions 自己的 shell 契约(defaults.run.shell: 'bash'bash --noprofile --norc -eo pipefail)下执行,只 stub npm。全绿但被 RPC 饿死的那一轮:base 退出 1,head 退出 0 并发出 ::warning::,且后续 test:scripts leg 照常运行。真实测试失败、kill switch 关掉、junit 缺失、健康的 runner —— 两侧都保持红/绿一致。

用真实 vitest 字节做的验证03-real-vitest-starved-rpc-refused.png):在本容器里真跑了两次 vitest 3.2.7。一次是逃逸的 write after end(archive-safety 那一类),4/4 测试通过、退出 1、junit 仍写出 failures="0";分类器拒绝它 —— PR 的核心前提(run 级 unhandled error 不进 junit,所以 failures 是权威信号)被真实产物证实。另一次通过阻塞主进程事件循环 65 秒,真的把 birpc 的 DEFAULT_TIMEOUT(= 6e4,已在 shipped dist 里核对)打爆 —— 但超时的 RPC 是 snapshotSaved,不是 onTaskUpdate,12/12 通过、退出 1,分类器拒绝了它。

findings

  1. (Suggestion,覆盖面) 同一个 60 秒预算下,worker 侧共有 ≥10 个 RPC 名字会产出 [vitest-worker]: Timeout calling "<name>",分类器只硬编码了其中一个。上面那次真实饿死恰好命中 snapshotSaved,形状与 PR 要容忍的完全一致却被拒。fail-closed,因此不是回归;但标题所说的「a starved vitest worker RPC」这一并未被覆盖。附已实测的候选修法(allowlist,27/27,零附带损伤)。
  2. (Suggestion,测试覆盖) 签名字面量没有被钉住:测试文件的 fixture 是用 RPC_TIMEOUT_SIGNATURE 本身拼出来的,把常量改错后 16 个测试全绿。已界定后果:签名漂移是 fail closed,只会让 job 重新变红,绝不会造成误放行。
  3. (Suggestion,测试覆盖) normalizeLog 的 ANSI 剥离是承重的但没被钉住:删掉它 16 个测试全绿,而一行带颜色的 npm error pathNO_COLOR 缺席时 npm 自己的输出)在有剥离时放行、无剥离时被拒。归类为覆盖缺口,不是死代码。

未覆盖范围:PR 最有力的证据(13 份真实 job 日志回放,放行 5 / 拒绝 8)无法在本轮校准 —— 容器内没有 GitHub token 也没有网络,取不到那些日志;改用本地捕获的真实产物字节替代。此外:真实 ECS 池 load 218–270 未复现(probe B 复现的是机制而非成因);shellcheck / actionlint 容器内没有也装不上(只跑了 bash -n);未做与当前 main 的 trial merge;未端到端跑真实的多小时 test:ci:workspaces,因此 tee 落盘的日志体积未实测。

Scope

Central claim. The Linux Test job stops going red when the only thing that broke is a
starved vitest onTaskUpdate worker RPC and every test passed — and stays red for everything
else.

Secondary claims. (SC1) every branch fails closed, so the tolerance is by signature rather
than by a whole-run boolean, and an escaping product exception is never tolerated; (SC2) the
wiring is correct — the log is captured, the kill switch reverses it, a tolerated run emits one
::warning::, and the remaining test legs still run.

Out of scope by choice: the pool capacity itself (#10879); extending junit reporters to the
other 19 workspaces (the PR defers this explicitly); the macOS/Windows legs, which already get
the exemption via dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux'.

A/B table — the load-bearing proof

Both arms are the run: block of the step "Run tests and generate reports" in job test,
extracted verbatim by YAML parse (base 2565 bytes, head 3499 bytes) and executed under
the step's own shell contract. Only npm is stubbed: it emits a captured log and a chosen exit
code. node, mktemp, tee, df, pgrep and the classifier are real, RUNNER_OS=Linux so
the real TMPDIR routing and EXIT trap run, and DISK_SAMPLES is pre-seeded so peakLoad is
deterministic. Cells c6/c7 feed bytes captured from real vitest 3.2.7 runs in this container,
plus the real junit.xml those runs wrote.

Witness: 01-ab-run-block-base-vs-head.png. Raw per-cell stdout/stderr: logs/ab-cells/.

cell scenario oracle base head head ::warning:: test:scripts still runs
c1-rpc-only all-green suite, exit 1 on onTaskUpdate starvation only step exit code 1 0 yes yes
c2-rpc-plus-fail a real FAIL rides along with the RPC timeouts step exit code 1 1 no no
c3-kill-switch QWEN_CI_TOLERATE_RPC_TIMEOUT=0 step exit code 1 1 no no
c4-missing-junit a blamed workspace wrote no junit (OOM/segfault shape) step exit code 1 1 no no
c5-clean-pass healthy runner, nothing to classify step exit code 0 0 no yes
c6-real-escaping-error real vitest bytes: escaping write after end, junit failures="0" step exit code 1 1 no no
c7-real-rpc-sibling real vitest bytes: all-green, exit 1, starved worker RPC (snapshotSaved) step exit code 1 1 no no

cells that flipped base->head: ["c1-rpc-only"] — exactly one, and it is red → green. 47/47
assertions in this harness.

The emitted annotation on the flipping cell, verbatim:

::warning::Test step tolerated a runner-capacity artifact: on ecs-qwen-hk5-2, peak host load 247.75, 4 vitest worker "onTaskUpdate" RPC timeout(s) with 0 failing tests (packages/cli, packages/core). Every test passed; the exit came from worker IPC starving on CPU. The pool is over capacity — see #10879.

Every field the description promises is present (runner, peak DFSAMPLE load, suppressed
timeout count, #10879), so a tolerated run is not silent. SC1 and SC2 both hold.

Real-vitest oracle (the premise condition 5 rests on)

The description calls condition 5 "load-bearing" on the claim that vitest always writes
tests/failures/errors/time and never increments errors, so run-level unhandled errors
do not reach junit at all
. That is a claim about a third-party producer, so it was measured
against the shipped one rather than read. Two real runs, dangerouslyIgnoreUnhandledErrors: false (i.e. the Linux configuration), junit reporter on:

probe what really happened exit tests junit written <testsuites> totals
A — escaping rejection a real write after end escaping an unawaited async context 1 4 passed (4) yes tests="4" failures="0" errors="0"
B — starved worker RPC main event loop blocked 65 s inside a reporter, so a worker RPC exceeded birpc's budget 1 12 passed (12) yes tests="12" failures="0" errors="0"

Both confirm the premise exactly: an all-green suite exits 1 on a run-level unhandled error,
junit is still flushed, and the unhandled error does not reach failures or errors. The
tally wording is also confirmed verbatim against its producer
(vitest/dist/chunks/cli-api.DVe0nWUx.js:5582): `Vitest caught ${errors.length} unhandled error${errors.length > 1 ? "s" : ""} during the test run.` — singular and plural, wrapped in
c.red(c.bold(...)), which is why normalizeLog's ANSI stripping and the errors? alternation
both matter. The "fixed 60s budget" claim is confirmed too: DEFAULT_TIMEOUT = 6e4 at
vitest/dist/chunks/index.B521nVV-.js:3, consumed by the bundled birpc.

Witness: 03-real-vitest-starved-rpc-refused.png. Raw logs and junit: logs/real-vitest-probes/.

Corrections to the description

These are corrections to the text, not requests to change code. Each was measured; the gate runs
and the numeric censuses behind items 2–6 are captured in
05-gates-and-pr-numeric-claims.png (33/33 assertions).

  1. The title names a class; the code covers one member of it. "…on a starved vitest worker
    RPC alone" reads as the whole failure class, and the body reinforces that ("onTaskUpdate is
    vitest's own worker IPC"). The code tolerates exactly one signature. The shipped producer
    (vitest/dist/chunks/rpc.-pEldfrD.js:49) builds the message from a variable:
    `[vitest-worker]: Timeout calling "${functionName}"`, and a census of rpc().<name> call
    sites in vitest 3.2.7 finds 10 such names sharing that one 60 s budget. Probe B hit
    snapshotSaved — see Finding 1. The "Risk & Scope" section bounds the false-tolerance side
    of the signature but never mentions the false-refusal side.
  2. "Of the 27 workspaces, 22 carry a test:ci script" — measured 25 active workspaces.
    The 27 counts the two !-excluded globs (packages/desktop-shell, packages/live-host),
    which exist on disk but are excluded from the npm workspace set. The substantive part of the
    claim is exactly right and reproduces: 22 with test:ci, 3 with a junit reporter, so
    19 have nothing for condition 5 to read and refuse. Confirmed too that all three junit
    destinations really resolve to <workspace>/junit.xml — including web-shell, whose
    root: 'client' + outputFile.junit: '../junit.xml' lands back inside its own directory, so
    the classifier's join(workspace, 'junit.xml') is correct for all three.
  3. "24 of the 25 HELPER_TESTS → 426 tests / 422 pass" with qwen-triage-workflow.test.mjs
    excluded as a macOS bash-3.2 artefact — confirmed, and the excluded gate passes here. On
    Linux/bash 5.2 all 25 files run 551 tests / 551 pass / 0 fail; that file alone is
    125/125; and 551 − 125 = 426, matching the author's 24-file count exactly. The
    bash-version diagnosis was correct.
  4. "check-workflow-size.sh … its arithmetic was checked by hand" — it runs here, and it
    passes by execution (exit 0), so the hand arithmetic is now machine-confirmed: ci.yml =
    134204 bytes, recorded baseline 131378, growth 2826 of the 4096 allowance. All
    four numbers in the description are exact.
  5. The guard-suite counts do not reproduce. For the five named vitest patterns the
    description reports "Test Files 6 passed, Tests 130 passed | 2 skipped"; measured here
    5 files / 115 passed / 0 skipped. Those five names match exactly five files in this tree.
    Green either way — noted only so the next reader does not chase the delta.
  6. "the block's TMPDIR routing (lines 16–22) precedes the new WORKSPACES_LOG assignment
    (line 40)"
    — confirmed line-for-line against the extracted head block: export TMPDIR at
    16, trap 'rm -rf "$TMPDIR" …' EXIT at 22, WORKSPACES_LOG="${TMPDIR}/test-ci-workspaces.log"
    at 40. The captured log therefore lands in the disk-backed temp the EXIT trap already cleans,
    as claimed.

Findings

Ordered by severity. None is a merge blocker: every one is fail-closed, so the worst outcome is
a red job — today's behaviour — never a false green.

1. (Suggestion) The tolerance covers 1 of ≥10 worker RPC names that share the same 60 s budget

Reproduce — the probe config that produced the real bytes is kept in this artifact dir, so the
run is reproducible without re-deriving it (logs/real-vitest-probes/probe-b-config/):

cp -r tmp/pr11103-verify-20260905-142953/logs/real-vitest-probes/probe-b-config /tmp/rpcprobe-b
cd /tmp/rpcprobe-b && npx vitest run --config vitest.config.mts   # exit 1, 12/12 tests passed
grep -E 'Timeout calling|Tests |EXIT' run.log
#   Error: [vitest-worker]: Timeout calling "snapshotSaved"
#        Tests  12 passed (12)
#   EXIT=1

# the same bytes, driven through the real classifier the way ci.yml drives it:
cd <repo> && node .github/scripts/ci/classify-infra-flake.mjs \
  --log tmp/pr11103-verify-20260905-142953/logs/ab-cells/fixtures/c7-real-rpc-sibling.log \
  --root tmp/pr11103-verify-20260905-142953/logs/ab-cells/cell-c7-real-rpc-sibling-head
#   exit 1 — infra-flake: not tolerated — log carries no "[vitest-worker]: Timeout calling "onTaskUpdate""

The captured log and the real junit that run wrote are both in this artifact dir
(logs/ab-cells/fixtures/c7-real-rpc-sibling.log, logs/real-vitest-probes/probe-b-junit.xml).
That is the exact shape the PR exists to tolerate — every test passed, exit 1, junit
failures="0", the exit caused by nothing but vitest's own worker IPC — and it is refused.

Blast radius. The mechanism is one shared constant, not one call site: DEFAULT_TIMEOUT = 6e4 in the bundled birpc, and the producer formats whatever method was in flight. Census of
rpc().&lt;name> in vitest 3.2.7 (10 names): fetch, getCountOfFailedTests, onAfterSuiteRun,
onCancel, onCollected, onQueued, onTaskAnnotate, onTaskUpdate, resolveId,
snapshotSaved. Driven through the real classify(), exactly one is tolerated. Three further
channels use the same wording with a different prefix and are all refused: [vitest-pool]
(4 producer sites), [vitest-api], [vitest-browser]. Witness:
04-sibling-sweep-and-scaling-ladder.png; 22/22 assertions.

Which scary readings do NOT hold. This cannot cause a false tolerance — it is strictly
fail-closed. And the dangerous siblings are correctly excluded today: an onUnhandledError
timeout (a real error whose delivery is what failed) and the module-graph RPCs fetch /
transform / resolveId (the code under test never loaded) all refuse. Any broadening must keep
them refusing, which is why the fix below is an allowlist and not a prefix match.

Measured candidate fix (optional follow-up, not required to merge)

Curated allowlist of progress-reporting RPCs, replacing the single hardcoded signature. Kept
verbatim at logs/suggested-fix/patched-classify-infra-flake.mjs; harness
harnesses/suggested-fix-harness.mjs.

const TOLERATED_RPC_METHODS = new Set([
  'onTaskUpdate', 'onCollected', 'onAfterSuiteRun', 'onQueued',
  'onTaskAnnotate', 'onCancel', 'snapshotSaved',
]);
const RPC_TIMEOUT_PATTERN = /\[vitest-worker\]: Timeout calling "([A-Za-z]+)"/g;

export function countRpcTimeouts(logText) {
  let total = 0;
  for (const match of normalizeLog(logText).matchAll(RPC_TIMEOUT_PATTERN)) {
    if (TOLERATED_RPC_METHODS.has(match[1])) total += 1;
  }
  return total;
}

Measured, 27/27 assertions, three results quoted as required:

  • hostile fixture goes clean — probe B's real starved-snapshotSaved run flips
    tolerated=false → true.
  • benign fixture byte-identical / zero collateral — probe A's real escaping
    write after end run still refuses; all 9 dangerous siblings (onUnhandledError ×2, fetch,
    transform, resolveId, getCountOfFailedTests, [vitest-pool], [vitest-api],
    [vitest-browser]) still refuse; conditions 2–5 still bite (real FAIL line, tally mismatch,
    missing junit, failures != 0).
  • the affected suite's counts are unchanged — the PR's own 16 tests: 16 pass / 0 fail, both
    unpatched and patched.

That last line is the unpinned-axis signal, not reassurance: the suite cannot tell head from
head-plus-fix
, because its fixtures are built from the constant itself. Shipping this fix
should ship the two fixtures that would go red — a literal-signature case and a
snapshotSaved case (Finding 2).

2. (Suggestion) The signature literal is unpinned — the suite is green against a wrong signature

Reproduce:

node tmp/mutation-harness.mjs    # mutant PC1-control-signature-text
#   [PC1-control-signature-text] POSITIVE CONTROL: signature string changed to a wrong method name
#     tests=16 pass=16 fail=0 exit=0 => SURVIVED

Renaming the constant's value (onTaskUpdateonTaskUpdateTYPO) leaves all 16 tests green.
The cause is structural: the test file imports RPC_TIMEOUT_SIGNATURE and builds its fixtures
from it (const RPC_ERROR = \${TS}Error: ${RPC_TIMEOUT_SIGNATURE}``), so a mutation moves the
fixture and the matcher together. No assertion anywhere in the file mentions the literal text
(checked).

Bounded. Measured, not argued: with a drifted signature the real shape is
tolerated=false — it fails closed. So the consequence is silent obsolescence (the classifier
becomes a no-op and jobs go red again, with only a not tolerated stderr line to explain it),
never a false tolerance. That is a much smaller risk than the signature being the whole
allowlist suggests, and it is worth saying plainly so the finding is not over-read.

Fix. One test on the literal, ideally pinned to the producer so a vitest bump breaks it
loudly instead of silently. Measured: exactly 1 file in the shipped node_modules/vitest/dist
contains the producer string — chunks/rpc.-pEldfrD.js. A test asserting
RPC_TIMEOUT_SIGNATURE === '[vitest-worker]: Timeout calling "onTaskUpdate"', plus (optionally)
that the shipped dist still contains that literal, would pin both the value and its upstream.

3. (Suggestion) normalizeLog's ANSI stripping is load-bearing but unpinned — a coverage gap

Reproduce:

node tmp/mutation-harness.mjs      # G7-normalize-ansi => SURVIVED (16/16 green)
node tmp/survivor-harness.mjs      # adjudicates it
#   [coloured npm error path]
#     unmutated: workspaces=["packages/cli"] tolerated=true
#     G7 mutant: workspaces=[]              tolerated=false  (npm named no failing workspace…)
#     => ANSI stripping is LOAD-BEARING (unpinned = coverage gap)

npm colourises its own npm error … lines when NO_COLOR is absent — a local run, or a log
fetched later through the API, which the module's own docblock says it supports. With the strip,
a magenta-wrapped path yields packages/cli; without it, NPM_ERROR_PATH_PATTERN's \S+$
anchor hits the trailing SGR reset and the workspace list comes back empty, so a run that should
be tolerated is refused.

Classified as a coverage gap, not dead code and not redundant defence — the behaviour is
right and reachable, nothing asserts it. Note the asymmetry the matrix exposes: removing the
timestamp half of the same normalizeLog line kills 7 tests, while removing the ANSI half
kills 0. The test named normalizes ANSI escapes and Actions timestamps is pinned entirely by
its timestamp fixture; its npm error path line is undecorated, so its name over-claims the ANSI
half. Decorating that one line would close the gap.

A second, benign sibling measured for contrast: a trailing space after the npm path also
loses the workspace (workspaces=[]) — fail-closed, cosmetic, no action needed.

4. (Note) The kill switch only honours the literal 0

ci.yml:772: if [ "$RC" -ne 0 ] && [ "${QWEN_CI_TOLERATE_RPC_TIMEOUT:-1}" != '0' ]; then. Only
the exact string 0 disables the tolerance; an operator who sets the repository variable to
false, no or off — the natural guess, and the value the neighbouring VITEST_RETRY knob
accepts as 'off' — leaves it enabled with nothing saying so. The in-file comment does say
"Set the repository variable to '0'", so this is a robustness nit rather than a defect; the A/B
c3 cell confirms '0' itself works.

5. (Note) The size ratchet leaves ~1.2 KB for the next ci.yml PR

Verified by execution, not arithmetic: gate exit 0. But the recorded baseline (131378) is now
1384 bytes behind HEAD^1 (132762) — main had already grown past it before this PR. This PR adds
1442, taking total growth to 2826 of 4096 and leaving 1270 bytes. Not a defect in this PR and
not its job to fix; flagged because the ratchet is close to forcing a baseline bump on whichever
ci.yml change lands next.

Mutation matrix (all 14 mutants, survivors included)

Witness: 02-mutation-matrix-guards.png. Per-mutant raw logs: logs/mutation-matrix/.
Each mutation disables exactly one guard in a scratch copy, interface-preserving (no export
removed), so a red test proves a failed assertion — every kill was checked to be
AssertionError-shaped rather than an import or compile break.

mutant what it disables result classification
G1 cond 1 — no RPC signature in the log KILLED (1 red) pinned
G2 cond 2 — vitest tally must equal signature count KILLED (1 red) pinned
G3 cond 3 — any real FAIL line KILLED (1 red) pinned
G4 cond 4 — npm must name a workspace KILLED (1 red) pinned
G5a cond 5 — blamed workspace wrote no junit KILLED (2 red) pinned
G5b cond 5 — absent counters refused, not read as zero KILLED (1 red) pinned
G5c cond 5 — junit failures != 0 KILLED (1 red) pinned
G6 no junit reader supplied KILLED (1 red) pinned
G7 normalizeLog ANSI stripping SURVIVED coverage gap (Finding 3)
G8 normalizeLog timestamp stripping KILLED (7 red) pinned
C1 cond 2 and cond 3 together KILLED (2 red) layered set is load-bearing
C2 cond 2 and 3 and 5c together KILLED (3 red) all three layers load-bearing
PC1 control: signature text changed SURVIVED self-referential fixture (Finding 2)
PC2 control: peakLoad returns null KILLED (2 red) harness is live

Guards killed 9/10; combinations 2/2; controls 1/2.

The unmutated control is green (16 tests / 16 pass / 0 fail), so the kills mean something, and
PC2's kill proves the runner collected the mutated file — the two survivors are findings about
the suite, not about the harness. On the layered-defence question: conditions 2, 3 and 5c defend
one hazard from three directions, and each is killed alone, so no combination row is needed to
prove the set load-bearing — but C1/C2 were run anyway and confirm no layer is masking another.

Sibling sweep and scaling ladder (22/22)

Witness: 04-sibling-sweep-and-scaling-ladder.png. Beyond Finding 1's RPC census: all three
other-channel prefixes refuse; singular and plural tallies both parse; a nested workspace
(packages/channels/base) attributes correctly; a checkout path that itself contains
/packages/ is handled correctly because .* is greedy and takes the last segment.

CRLF: predicted refusal, measured as handled. An API-fetched log with \r\n line endings
yields the same verdict, the same workspace attribution, and no \r leaked into the captured
path — JS multiline $ matches before the \r. My prediction was wrong and the measurement
corrected it; the invariant (CRLF ≡ LF) is what the harness now asserts.

Scaling ladder — no superlinear behaviour. The classifier is a parser over untrusted text of
unbounded size, so it was probed with a ladder under timeout, not a single sample:

rung actual bytes classify() ms/MB verdict
1 MB 1,049,088 86 ms 85.9 tolerated
5 MB 5,243,481 384 ms 76.8 tolerated
20 MB 20,971,981 1,256 ms 62.8 tolerated
100 MB 104,858,325 7,111 ms 71.1 tolerated

Flat ms/MB across a 100× span and an identical verdict at every rung: linear, no ReDoS-shaped
blowup, and even a 100 MB log classifies in ~7 s — so the added step cannot itself time out. This
is a bounded negative, reported because the mechanism (several regexes over a multi-hour CI log)
is exactly where such a blowup would live.

Not covered

  1. The 13-run replay is uncalibrated here. The description's strongest evidence is a replay
    against thirteen real job logs from 2026-09-05 (five tolerated, eight refused, matching a
    manual classification). This container has no GitHub token and no reliable network, so
    those logs were not retrievable and the replay could not be run — meaning, per the
    workflow-PR rule, it could not be calibrated against a real artifact the production step
    emitted. There is no previous-report.md either (this is a first round). What would have
    calibrated it:
    the thirteen raw job logs mounted read-only into the container. What I did
    instead is stated as such throughout: real producer bytes captured locally, not the real
    production logs. The five/eight split and the per-run table are therefore unverified, and
    the two healthy hosted-runner control rows ("refused on condition 1 rather than condition 3")
    are unverified too — though c6 independently confirms condition 1 is what refuses a genuine
    escaping error.
  2. Shape, not cause. Probe B reproduces the mechanism (a worker RPC exceeding birpc's 60 s
    budget → run-level unhandled error → all-green suite exits 1 → junit still failures="0") by
    blocking the main event loop. It does not reproduce the cause (CPU contention at load
    218–270 on the shared pool). The live /proc/loadavg sample of 256.69 corroborates that the
    pool is really that loaded, but no test in this round ran the suite under contention.
  3. shellcheck and actionlint were not run — neither is installed in this container and
    neither could be installed offline. bash -n was run on both extracted blocks instead (both
    clean). Not implied as passing.
  4. No trial merge into current main. The checkout is depth 2 with no network, so main's tip
    is unreachable; the A/B is against HEAD^1 only. The base is one commit behind the merge
    commit and the diff touches three files, so the risk is low, but it is unmeasured.
  5. The real multi-hour npm run test:ci:workspaces was never executed end-to-end. The A/B
    stubs npm, so the interaction between a genuine 96-minute run and the new tee is
    unmeasured — specifically the on-disk size of WORKSPACES_LOG. This lane documents ENOSPC
    as a live failure mode and routes TMPDIR to /var/tmp for that reason, and this PR adds one
    full copy of a multi-workspace log there. Two things bound it and both were verified: the
    file's lifetime is one step (the trap 'rm -rf "$TMPDIR"' EXIT at block line 22 removes it,
    and the PR's claim that the TMPDIR routing precedes the assignment is confirmed line-for-line),
    and the classifier's own cost at 100 MB is ~7 s. The byte volume is not measured and is the
    one residual cost I could not price.
  6. junit staleness is only partly resolved. The classifier does not check junit freshness, so
    a stale junit.xml from a prior run could in principle satisfy condition 5 after a worker
    died — defeating the very protection the description claims for it. Measured locally:
    git clean -ffdx does remove a gitignored junit.xml (.gitignore:82), and the test
    job's checkout does not override clean:. What I could not verify offline is
    actions/checkout v6.0.3's own default for that input — no network, and the rule is to read a
    third-party action's manifest rather than assume. So the residual (a reused self-hosted
    workspace and a checkout that does not clean) is unresolved; it is narrow and I found no
    evidence it holds.
  7. No mutation of ci.yml itself. The mutation matrix covers the classifier module. The
    shell wiring was covered by A/B cells (including the kill switch, c3) rather than by mutants,
    and the ${{ vars.… || '1' }} expression was not exercised against a real repository variable
    — only its shell-side consequence was.
  8. Not a repo-wide gate. Only the affected surfaces were run: the new test file, all 25
    HELPER_TESTS, the five named vitest guard files, the size ratchet, and eslint/prettier on the
    three changed files. packages/cli and packages/core unit suites were not run — the diff
    touches no production code under packages/.

Methodology

Everything ran in this CI container at the merge-ref checkout (HEAD = merge, HEAD^1 = base
tip, HEAD^2 = PR head, depth 2), with the pre-existing npm ci + npm run build — no rebuild
was needed, because the changed surface is a Node ESM script and a shell block, neither of which
crosses a workspace boundary; main-failure-signature.mjs, the one internal dependency the
classifier imports, is unchanged by this PR (verified with git diff --stat over it), so
there is no head/base link confound to assert against. The A/B extracted both run: blocks by
YAML parse and executed them under bash --noprofile --norc -eo pipefail, the contract
defaults.run.shell: 'bash' (ci.yml:72) selects, in scratch roots containing a real copy of the
classifier and real junit artifacts, with only npm stubbed. The real-vitest probes drove the
shipped vitest 3.2.7 directly — one via an escaping rejection, one via a custom reporter that
blocks the main event loop for 65 s to starve a worker RPC — and fed the resulting logs and
junit files into the real classifier through the same command line ci.yml uses. Mutation,
survivor, sibling and candidate-fix harnesses each ran the PR's own unmodified test file or the
real classify() against a scratch copy; the gates harness re-ran the cheap gates live and
asserted the long ones (HELPER_TESTS, the vitest guards, qwen-triage-workflow.test.mjs) from
the raw logs of real executions, and proved the eslint gate live by planting an unused variable
in the same directory before citing its clean result. Harnesses are in harnesses/, raw per-cell
and per-mutant logs in logs/, and the five captures in evidence/. Assertion counts are the sum
of the six harness summaries: A/B 47/0, mutation 29/2, survivors 7/0, siblings 22/0, candidate fix
27/0, gates 33/0 — 165 pass / 2 fail of 167. Both failures are the two surviving mutants,
i.e. findings about the PR's test suite; no unexpected outcome was attributed to the PR's
runtime behaviour, and no harness failure was reclassified as a PR defect (three of my own
harness bugs — a grep missing -F, an ANSI-decorated log summary, and a broken npm query
census — were found, fixed, and re-run rather than reported).

Flakiness gate log

rounds=5 files=1 skipped=0
file .github/scripts/ci/classify-infra-flake.test.mjs: (cd .) node --test ./.github/scripts/ci/classify-infra-flake.test.mjs


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  .github/scripts/ci/classify-infra-flake.test.mjs: PPPPP

verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/ci/classify-infra-flake.test.mjs: P (exit 0)
round 2 · .github/scripts/ci/classify-infra-flake.test.mjs: P (exit 0)
round 3 · .github/scripts/ci/classify-infra-flake.test.mjs: P (exit 0)
round 4 · .github/scripts/ci/classify-infra-flake.test.mjs: P (exit 0)
round 5 · .github/scripts/ci/classify-infra-flake.test.mjs: P (exit 0)

Evidence images

01-ab-run-block-base-vs-head

02-mutation-matrix-guards

03-real-vitest-starved-rpc-refused

04-sibling-sweep-and-scaling-ladder

05-gates-and-pr-numeric-claims

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks — and thanks for turning the description around so quickly.

Template looks good now ✓ ## Risk & Scope carries all three bullets, ## Linked Issues is its own section, and the Chinese block is a real paragraph-by-paragraph translation rather than a summary. That satisfies the earlier request-changes, which was description-only.

Problem: observed, not theoretical. Five named run IDs (33953519066, 33952382944, 33952315316, 33948811958, 33948268415) went red with zero failing tests, and the eight refuse-cases include two healthy GitHub-hosted runners as a control. That is a measured failure with a denominator, which is more than most flake PRs bring.

Direction: aligned. A red Test job that no test caused is pure noise — it burns maintainer attention and trains people to re-run instead of read. CHANGELOG: no direct reference, but this is repo-internal CI rather than a product surface, so that is expected. The part I want to call out as right is the refusal to just flip dangerouslyIgnoreUnhandledErrors on Linux. I checked that against the tree instead of taking it on faith: packages/core/src/extension/archive-safety.test.ts documents the minipass write after end escaping as an uncaught exception, and — this is the load-bearing detail — that release failure (run 33576013293) had no FAIL line and 9480 tests passing. A whole-run boolean genuinely cannot tell that apart from a starved worker. Discriminating by signature can.

Size: not applicable — no core paths are touched (packages/core/src/**, packages/*/src/{auth,providers,models,config,tools,services}/**), so Stage 0's two-tier gate does not engage. For the record: 322 production lines (classify-infra-flake.mjs 299, ci.yml +20/−3) and 281 test lines, well under every threshold.

Approach: the scope feels right, and I went looking for the smaller version before reading the code in detail. I could not find one. Each of the five conditions earns its place, and condition 2 is the one that actually matters — conditions 3 and 5 would both have waved the archive-safety incident through, since it had no FAIL line and its junit would have read failures="0". Reusing extractFailingTests from main-failure-signature.mjs rather than writing a second FAIL parser is the right instinct. The peakLoad / --samples / runnerName threading is the only part not load-bearing for the verdict, and it is correctly built so a missing samples file cannot change the answer — it just keeps the suppression visible instead of silent, which is the point. Putting the kill switch in a repo variable rather than a code constant is right for the same reason: reversing this never needs a PR.

Risk: no elevated risk signals — none of the three files match the revert-correlated path set.

One thing to flag going into the code review, as a question rather than a concern: the tolerance can only reach packages/cli, packages/core and packages/web-shell, because those are the only three vitest configs carrying a junit reporter. You have documented that honestly in ## Risk & Scope, which is exactly where it belongs — I raise it only because a reader who assumes this covers all of test:ci:workspaces will be surprised the first time a channels/* leg starves and stays red.

Moving on to code review. 🔍

中文说明

感谢贡献——也感谢你这么快就把描述补齐了。

模板现在完整 ✓ ## Risk & Scope 三个要点都在,## Linked Issues 独立成节,中文区块也是逐段完整翻译而不是摘要。之前那次 request-changes 只是针对描述,现在已满足。

问题:是已观测到的,不是理论性加固。五个具名 run ID(33953519066、33952382944、33952315316、33948811958、33948268415)在零失败用例的情况下变红,而八个"拒绝"样本里还包含两台健康的 GitHub 托管 runner 作为对照。这是带分母的实测失败,比大多数 flake PR 拿出的证据更扎实。

方向:对齐。一个并非由任何用例导致的红色 Test job 就是纯噪音——它消耗维护者的注意力,还会训练大家去"重跑"而不是"去读"。CHANGELOG:没有直接对应条目,但这本来就是仓库内部的 CI,而非产品面,属于预期。我想特别指出做对了的一点:没有简单地在 Linux 上翻 dangerouslyIgnoreUnhandledErrors。这一点我没有采信描述,而是对着代码树核实过:packages/core/src/extension/archive-safety.test.ts 记录了 minipass write after end 以未捕获异常逃逸的情形,而关键在于——那次发布失败(run 33576013293)日志里没有 FAIL 行,且 9480 个用例全部通过。整轮级别的布尔开关确实无法把它和"worker 被饿死"区分开,按签名区分才可以。

规模:不适用——没有触及核心路径(packages/core/src/**packages/*/src/{auth,providers,models,config,tools,services}/**),因此 Stage 0 的两级门禁不生效。仅作记录:生产代码 322 行(classify-infra-flake.mjs 299 行,ci.yml +20/−3),测试 281 行,远低于各项阈值。

方案:范围合理。在细读代码之前我先去找过"更小的版本",没找到。五个条件各有其必要性,其中真正关键的是条件 2——条件 3 和条件 5 都会放过 archive-safety 那次事故,因为它既没有 FAIL 行,junit 也会是 failures="0"。复用 main-failure-signature.mjs 里的 extractFailingTests 而不是再写一个 FAIL 解析器,这个直觉是对的。peakLoad / --samples / runnerName 这一串是唯一对判定结果不起决定作用的部分,而且实现得当:samples 文件缺失也不会改变结论——它只是让"被容忍"这件事保持可见而非静默,这正是它的意义。把开关放在仓库变量而不是代码常量里,出于同样的理由是对的:回退这个行为不需要提 PR。

风险:无升级风险信号——三个文件都不匹配与 revert 相关的路径集合。

进入代码审查前有一点想提出来,是疑问而非顾虑:这个容忍逻辑实际只可能作用于 packages/clipackages/corepackages/web-shell,因为只有这三个 vitest 配置带了 junit reporter。你在 ## Risk & Scope 里已经如实写明,位置也完全正确——我之所以还要提,是因为如果有人以为它覆盖了整个 test:ci:workspaces,那么第一次遇到 channels/* leg 被饿死却依然变红时会感到意外。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0e97aa165c9a035962d64e603e89b2db39120d94 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Static review only — this run never executes PR-derived code, so the testing evidence below is the PR's own CI, read through the API, plus what I verified by reading the base tree. Nothing here is re-run locally.

Code review

No correctness or security blockers. Two non-blocking notes.

1. The wrapped-OS-job guard still passes, but no longer pins the capture that matters on this leg (Suggestion). scripts/tests/no-ak-integration-ci.test.jspreserves test failures in every wrapped OS job walks ['set +e', 'npm run test:ci', 'RC=$?', 'set -e', 'pkill …', 'kill …', 'exit "$RC"'] by indexOf and asserts the ordering. It still passes, because the first literal RC=$? in the modified test step is now the inner one capturing npm run test:scriptsRC=${PIPESTATUS[0]} does not contain that substring — and the ordering holds regardless.

The intent of that assertion, though, was "the npm exit code is captured and propagated to exit "$RC"", and on this leg that is no longer what it checks. The regression this shape newly enables is someone writing RC=$? straight after the pipeline, which captures tee's status (near-always 0) and turns the Linux Test job permanently green regardless of test outcomes. That edit would still satisfy every assertion in the guard. Cheap fix: pin RC=${PIPESTATUS[0]} for the test job, or assert the npm line and its capture are adjacent.

2. The captured log is a new unbounded consumer of the routed $TMPDIR (observation). WORKSPACES_LOG="${TMPDIR}/test-ci-workspaces.log" puts the whole merged cli+core output on the filesystem this same step already samples for ENOSPC/inode pressure (#10035). The choice is defensible — on Linux $TMPDIR is disk-backed /var/tmp and the existing EXIT trap cleans it, while $RUNNER_TEMP (where $DISK_SAMPLES lives) would outlive the step — and the failure mode is benign: if tee cannot write, the log truncates, the trailing npm error path lines go missing, and the classifier refuses. So disk pressure costs you tolerance, never correctness. Worth a conscious decision rather than a default, that is all.

What I verified against the base tree

  • Condition 5's stale-junit hole is closed. This was the one assumption that could have turned a real break green: an OOM-killed worker writes no junit, so had a previous job's junit.xml survived on a self-hosted runner, condition 5 would have read it and tolerated. junit.xml is gitignored (.gitignore:82) and the test job's actions/checkout sets no clean: input, so the default clean: true runs git clean -ffdx and removes it before the suite runs.
  • packages/web-shell junit resolves where the classifier expects. outputFile.junit: '../junit.xml' looks wrong at a glance, but that config sets root: 'client', so the report lands at packages/web-shell/junit.xml — matching join(workspace, 'junit.xml') for all three junit-bearing workspaces.
  • 2>&1 is load-bearing, not decorative. npm error path is stderr; without the merge, condition 4 never sees a workspace and every run refuses.
  • The TMPDIR byte-identity pin across the three legs survives. That test slices from export TMPDIR= to the ( while true sampler sentinel; the new lines land after the sentinel, outside the pinned region.
  • The HELPER_TESTS addition is safe. ci-platform-lanes.test.js only asserts that specific entries are present and that helper steps run --test-concurrency=1; adding an entry breaks neither. The variable is shared by both the github_ci_only and full-profile helper steps, so one addition covers both paths as the comment above it requires.
  • The size ratchet, computed independently rather than taken from the description: ci.yml is 134204 bytes at head against 132762 at the base commit (+1442), and 2826 over the 131378 recorded in .github/workflows/.size-baseline — inside the 4096 GROWTH_ALLOWANCE and far inside the 470000 gate.
  • Every refusal branch I could construct refuses. Truncated log loses the npm error path tail → refuse. Missing tally → 0 !== N → refuse. A second kind of unhandled error → mismatch → refuse. An npm error path outside packages/ → no workspace → refuse. The asymmetry is right throughout: failure modes cost tolerance, never a false green.

Not verified: the shell wiring end-to-end on a real Linux runner — see the lane line below. The thirteen-run replay table in the description is the author's offline classification, attributed as their claim; this review did not re-run it.

CI evidence

The PR's own checks on 0e97aa165c9a035962d64e603e89b2db39120d94, fetched once via the check-runs API. Lint & Static is the meaningful green: it carries the workflow-size gate, ESLint, Prettier, typecheck and the HELPER_TESTS list this PR extends, so the new .mjs pair lints and its 16 tests run inside it. The macOS and Windows Test legs are skipped for this PR, which is consistent with only the Linux leg calling the classifier. Test (ubuntu-latest, Node 22.x) — the leg this PR actually modifies — was still running at review time; the table below is updated in place once CI settles.

Final CI results for 0e97aa1 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
web-shell E2E Smoke (ubuntu-latest, Node 22.x) 🚫 cancelled
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle this: @qwen-code /verify — the classifier's decision logic is well covered by its own tests, but the shell wiring is not observable from the diff, and a green Test (ubuntu-latest) will not exercise it either, because the tolerate path only fires under real CPU starvation. Specifically unproven end-to-end: that 2>&1 | tee plus RC=${PIPESTATUS[0]} still propagates a genuine npm failure to exit "$RC", and that a tolerated run falls through to npm run test:scripts instead of skipping it. A /verify run is already in flight on this PR — that report is where to look for it.

中文说明

本轮为纯静态审查——不会执行任何 PR 带来的代码,因此下面的测试证据来自 PR 自身的 CI(通过 API 读取)以及我对照基线代码树核实的内容,没有任何本地重跑。

代码审查

没有正确性或安全性阻塞项。两条非阻塞意见。

1. 三个 OS job 的包装守卫仍然通过,但在本 leg 上已不再钉住真正关键的捕获语句(建议)。scripts/tests/no-ak-integration-ci.test.js 里的 preserves test failures in every wrapped OS jobindexOf 依次检查 ['set +e', 'npm run test:ci', 'RC=$?', 'set -e', 'pkill …', 'kill …', 'exit "$RC"'] 并断言顺序。它依然通过,因为修改后的 test step 中第一个字面量 RC=$? 已经是内层那个(捕获 npm run test:scripts)——RC=${PIPESTATUS[0]} 不包含该子串——顺序断言因此照样成立。

但这条断言的本意是"npm 的退出码被捕获并传递到 exit "$RC"",而在本 leg 上它检查的已不是这件事。这种写法新引入的退化风险是:有人把 RC=$? 直接写在管道之后,那捕获到的是 tee 的状态(几乎总是 0),于是 Linux Test job 无论用例结果如何都会永久变绿。而这样的改动仍能通过该守卫的每一条断言。修法很便宜:为 test job 钉住 RC=${PIPESTATUS[0]},或断言 npm 那一行与其捕获语句相邻。

2. 捕获日志成了已路由 $TMPDIR 上一个新的、无上界的占用者(观察)。WORKSPACES_LOG="${TMPDIR}/test-ci-workspaces.log" 把 cli+core 的全部合并输出写到了同一个 step 已经在为 ENOSPC/inode 压力做采样的文件系统上(#10035)。这个选择是站得住的——Linux 上 $TMPDIR 是磁盘支撑的 /var/tmp,且已有 EXIT trap 会清理,而 $DISK_SAMPLES 所在的 $RUNNER_TEMP 会活过整个 step——并且失效方式是良性的:tee 写不进去时日志被截断,末尾的 npm error path 行丢失,分类器随即拒绝。所以磁盘压力只会让你失去"容忍",不会损害正确性。只是值得做一次有意识的决定,而不是顺手默认。

我对照基线代码树核实过的内容

  • 条件 5 的"陈旧 junit"漏洞是封住的。 这是唯一一个可能把真实故障放绿的假设:被 OOM 杀掉的 worker 不写 junit,因此如果自托管 runner 上残留了上一个 job 的 junit.xml,条件 5 就会读到它并予以容忍。junit.xml 已在 .gitignore:82 中被忽略,且 test job 的 actions/checkout 没有设置 clean: 输入,因此默认的 clean: true 会执行 git clean -ffdx,在用例运行前将其清除。
  • packages/web-shell 的 junit 落在分类器预期的位置。 outputFile.junit: '../junit.xml' 乍看是错的,但该配置设置了 root: 'client',所以报告落在 packages/web-shell/junit.xml——对三个带 junit 的 workspace 都能与 join(workspace, 'junit.xml') 对上。
  • 2>&1 是承重的,不是装饰。 npm error path 走 stderr;不做合并的话条件 4 永远看不到 workspace,所有运行都会被拒绝。
  • 三个 leg 之间 TMPDIR 逐字节一致的那条钉住断言得以保留。 该测试从 export TMPDIR= 切到 ( while true 采样器哨兵为止;新增行落在哨兵之后,处于被钉区域之外。
  • HELPER_TESTS 的新增是安全的。 ci-platform-lanes.test.js 只断言特定条目存在、以及 helper step 带 --test-concurrency=1;新增条目两者都不破坏。该变量由 github_ci_only 与 full-profile 两个 helper step 共用,因此按其上方注释的要求,一处新增即覆盖两条路径。
  • 体积棘轮为独立计算,而非采信描述:head 上 ci.yml 为 134204 字节,基线 commit 为 132762(+1442),相对 .github/workflows/.size-baseline 记录的 131378 增长 2826——在 4096 的 GROWTH_ALLOWANCE 之内,也远在 470000 的门禁之内。
  • 我能构造出的每一条拒绝分支都确实拒绝。 日志截断会丢掉末尾的 npm error path → 拒绝;tally 缺失 → 0 !== N → 拒绝;多出一类 unhandled error → 不相等 → 拒绝;npm error path 不在 packages/ 下 → 无 workspace → 拒绝。整体不对称性是对的:失效只会损失容忍度,不会造成假绿。

未验证: 真实 Linux runner 上 shell 接线的端到端行为——见下方的验证通道说明。描述中那份十三次运行的回放表是作者的离线分类结论,此处按作者自述引用,本审查未重跑。

CI 证据

针对 0e97aa165c9a035962d64e603e89b2db39120d94,通过 check-runs API 一次性读取。Lint & Static 是有意义的绿灯:它承载体积门禁、ESLint、Prettier、typecheck 以及本 PR 扩展的 HELPER_TESTS 列表,因此新增的这对 .mjs 通过了 lint,其 16 个用例也在其中运行。macOS 与 Windows 的 Test leg 对本 PR 是 skipped,这与"只有 Linux leg 调用分类器"一致。Test (ubuntu-latest, Node 22.x)——也就是本 PR 真正修改的那条 leg——在审查时仍在运行;上方表格会在 CI 结束后就地更新。

沙箱验证可以把这件事定下来:@qwen-code /verify——分类器的判定逻辑由它自己的测试覆盖得不错,但 shell 接线无法从 diff 观察,而且即便 Test (ubuntu-latest) 变绿也不会真正走到这条路径,因为容忍分支只有在真实 CPU 饥饿下才会触发。具体尚未端到端证实的是:2>&1 | teeRC=${PIPESTATUS[0]} 是否仍能把真实的 npm 失败传递到 exit "$RC",以及被容忍的运行是否会继续执行 npm run test:scripts 而不是跳过它。本 PR 上已有一次 /verify 在运行中——那份报告是查证这两点的去处。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0e97aa165c9a035962d64e603e89b2db39120d94 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, and it fails closed everywhere I could push on it. The one reservation is that the tolerate path has never actually fired in CI, so the shell wiring is proven by reading rather than by observation.

Stepping back: this is the right fix for a real, measured problem, built with the right instincts. Before reading the diff I wrote down what I would do — let the step fail, then classify the exit against the junit totals behind an env kill switch, reusing the existing FAIL parser instead of writing a second one. That is what you built, and I could not find a materially simpler version that keeps the discrimination. The obvious one-line alternative, flipping dangerouslyIgnoreUnhandledErrors on Linux, is the one you argued against, and the argument survives contact with the tree.

What actually convinced me is the asymmetry. Every branch I could think to push on refuses: truncated log, worker killed before writing junit, a second kind of unhandled error riding along, a real FAIL anywhere, a tally that goes missing, an npm error path outside packages/. The failure mode is always "stays red", never "goes green" — and for a change to a test gate, that is the only property that really matters. The design also puts its one load-bearing condition where it belongs: condition 2 is what separates a starved worker from the archive-safety class of real defect, and the test for it is built from that incident's actual numbers rather than an invented shape.

I checked the assumption that would have been the one real hole — condition 5 reading a stale junit.xml left behind by a previous job on a self-hosted runner. It is closed by the checkout's default clean plus the gitignore entry, and I have spelled that out in the Stage 2 comment because the whole OOM-killed-worker case you designed condition 5 for depends on it.

The two notes in Stage 2 are genuinely non-blocking: the guard test that pins the set +e … RC=$? … exit "$RC" wrapper still passes but no longer pins the capture that matters on this leg, and the captured log is a new unbounded writer on the $TMPDIR that same step samples for ENOSPC. Neither changes what this PR does today; the first is worth a follow-up because it is the guard that would have to catch a careless future edit.

Would I curse you or thank you in six months? Thank you, with one caveat. This treats the symptom and says so loudly — the ::warning:: names the runner, the peak load, and #10879 — which is the honest way to do it. But a warning only works if someone reads it. If the pool-capacity issue does not move, the lane slowly accumulates tolerated runs and the noise just changes shape rather than going away. That is a reason to keep #10879 warm, not a reason to hold this back.

Verdict: approve. Test (ubuntu-latest, Node 22.x) — the leg this PR modifies — is still running, so approval is deferred until CI lands green on 0e97aa165c9a035962d64e603e89b2db39120d94 rather than attested now.

One piece of housekeeping: my earlier request-changes on this PR was the description-only template gate, and the updated body satisfies it. PR reviews cannot be edited, so that review is still standing and reviewDecision still reads CHANGES_REQUESTED — worth dismissing if your branch protection counts it as a blocking vote alongside the approval below.

中文说明

信心度:4/5 —— 扎实,而且我每一处能施压的地方它都是 fail-closed 的。唯一的保留意见是:容忍分支至今没有在 CI 里真正触发过,所以 shell 接线是靠阅读而非靠观测证实的。

退一步看:这是针对一个真实且经过实测的问题所做的正确修复,直觉也对。在读 diff 之前我先写下了自己会怎么做——让 step 照常失败,然后在一个环境变量开关后面,拿 junit 汇总去分类这次退出,并复用已有的 FAIL 解析器而不是再写一个。你做的正是这个,而我没能找到一个既明显更简单、又能保住这种区分能力的版本。那个显而易见的一行替代方案(在 Linux 上翻 dangerouslyIgnoreUnhandledErrors)正是你反对的那个,而这个反对意见在对照代码树之后依然成立。

真正说服我的是那种不对称性。我能想到去施压的每一条分支都拒绝:日志被截断、worker 在写 junit 之前被杀、混进来另一类 unhandled error、任何位置出现真实 FAIL、tally 缺失、npm error path 不在 packages/ 下。失效方式永远是"保持红",绝不会是"变绿"——对于一个测试门禁的改动来说,这才是真正要紧的性质。设计也把它唯一承重的条件放在了该放的位置:条件 2 正是把"worker 被饿死"与 archive-safety 那类真实缺陷区分开的东西,而它的测试是用那次事故的真实数字构造的,不是凭空编的形状。

我核实了那个唯一可能成为真正漏洞的假设——条件 5 读到自托管 runner 上某个先前 job 残留的陈旧 junit.xml。它是封住的:checkout 的默认 clean 加上 gitignore 条目共同保证了这一点。我在 Stage 2 评论里把它写清楚了,因为你为条件 5 设计的整个"worker 被 OOM 杀掉"场景都依赖于此。

Stage 2 里的两条意见确实都是非阻塞的:钉住 set +e … RC=$? … exit "$RC" 包装的守卫测试仍然通过,但在本 leg 上已不再钉住真正关键的捕获语句;捕获日志则是那个 step 已在其上做 ENOSPC 采样的 $TMPDIR 上一个新的无上界写入者。两者都不改变这个 PR 今天的行为;第一条值得作为后续跟进,因为要抓住未来某次粗心改动,靠的正是这道守卫。

六个月后我会骂你还是谢你?谢你,但附一条提醒。这是在治症状,而且它大声地承认了这一点——::warning:: 会写出 runner 名、峰值负载和 #10879——这是诚实的做法。但警告只有在有人读的时候才有用。如果资源池容量问题不推进,这条 lane 会慢慢积累起被容忍的运行,噪音只是换了个形状,并没有消失。这是应该让 #10879 保持活跃的理由,而不是拦下本 PR 的理由。

结论:批准。Test (ubuntu-latest, Node 22.x)——也就是本 PR 修改的那条 leg——仍在运行,因此批准推迟到 CI 在 0e97aa165c9a035962d64e603e89b2db39120d94 上全绿之后,而不是现在就签发。

一点事务性说明:我此前对本 PR 的 request-changes 是那次仅针对描述的模板门禁,更新后的正文已满足要求。PR review 无法编辑,所以那条 review 仍然挂着,reviewDecision 也仍显示 CHANGES_REQUESTED——如果你们的分支保护会把它当作与下面这次批准并列的阻塞票,值得把它 dismiss 掉。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 0e97aa165c9a035962d64e603e89b2db39120d94 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The bot already has a review of its own on 0e97aa165c9a035962d64e603e89b2db39120d94, which still stands.

机器人在 0e97aa165c9a035962d64e603e89b2db39120d94 上已有自己的评审,且仍然有效。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 3)": shellcheck/actionlint on the newly added embedded shell lines (ci.yml:768-774) — neither binary is installed on this host (only yamllint ), so the repo's own p…; "agent 1d": none. I could not verify RPC_TIMEOUT_SIGNATURE against the installed vitest ( node_modules is absent in this worktree and in the parent checkout), so whether….

Not reviewed: reverse audit — stopped before round 5 by the review time budget.

Not linted (tool limitation, not a blocker): .github/workflows/ci.yml — actionlint embedded-shell source mapping is not yet supported.

Test Plan (not a blocker): ci/main-failure-signature.mjsno such file or directory.

— qwen3.8-max via Qwen Code /review (v0.23.0)

* to fix it (#10879).
*/

export const RPC_TIMEOUT_SIGNATURE =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-1: This is the repo's second — and by signature, third — divergent implementation of one tolerance policy, with no cross-reference between them. .github/workflows/release.yml:571-598 already tolerates a vitest worker Timeout calling on the same pool, in bash, on weaker proof and matching ANY method name; integration-tests/vitest.config.ts:50-51 already exempts this exact pool at config level. This file pins one RPC method and requires junit evidence.

Two costs, both measured. (1) A worker starves on a different vitest RPC — onCollected, onAfterSuiteRun: the release lane tolerates it and the release ships, while this classifier refuses and reds the required Test check on the identical runner fault. Two verdicts on one piece of evidence in one repository. vitest's timeout wrapper is generic over every awaited worker-to-collector call (onTimeoutError(functionName, args) special-cases fetch, transform, resolveId and onUnhandledError by name, proving they share the channel and its 60 s budget), and onAfterSuiteRun is awaited too — so on the push lane, where QWEN_CI_COVERAGE=1 turns coverage on, a run with 3 onTaskUpdate timeouts plus 1 starved onAfterSuiteRun refuses the WHOLE run with vitest counted 4 unhandled error(s) but only 3 are … — something else escaped, discarding the method name it already has in hand and naming a product exception that is not in the log. (2) A masking risk discovered and closed in one lane stays open in the other, and when vitest next rewords the message the fix must be found in both — one of them inside a 134 KB workflow file. The repo already treats this coupling as record-worthy: main-failure-signature.mjs:186-188 ("is re-implemented in bash/awk by … a fix to one must reach the other").

Witness:

the real release.yml step (extract-step --job workspace_tests, run verbatim with a stub npm) against the new classifier, fed THE SAME evidence — `Error: [vitest-worker]: Timeout calling "onCollected"` + ` Tests 10614 passed (10614)` + `npm error path …/packages/cli`, junit present with failures="0":
RELEASE LANE: ::warning title=Workspace tests passed through a Vitest transport timeout::Every test passed … Treated as a pass.
 release-step exit=0
CI LANE: infra-flake: not tolerated — log carries no "[vitest-worker]: Timeout calling "onTaskUpdate""
 classifier exit=1
neither site names the other (grep: release.yml never mentions classify-infra-flake; the new header never mentions release.yml)
corrections measured: the pinned set is 22 configs, not 24; 23 configs carry dangerouslyIgnoreUnhandledErrors — 22 exactly `process.platform !== 'linux'` and 1 divergent (integration-tests/vitest.config.ts:50-51 `|| isSelfHostedRunner`, "hit the same pressure class there (#10325), so self-hosted runners are exempted as well"), so the new header's "every vitest config already carries as process.platform !== 'linux'" is false for exactly the config that exempts this very pool

Match the sibling's method-agnostic form and record the coupling:

const RPC_TIMEOUT_PATTERN = /\[vitest-worker\]: Timeout calling "([A-Za-z]+)"/g;

Keep refusing on methods whose loss IS a real test failure (fetch, transform, resolveId, onUnhandledError — vitest appends the real error's text to that last one, so it carries information about the code under test), tolerate the results-channel ones the docblock's own criterion covers, and make the refusal enumerate the method names it saw rather than asserting "something else escaped". Then name both siblings in the header where it says "Nothing already in the lane covers it": release.yml's Timeout calling branch (same class, weaker proof, pinned by scripts/tests/release-workflow.test.js) and integration-tests/vitest.config.ts's self-hosted exemption, with the "a fix to one must reach the other" convention the repo already uses.

One constraint on the fix — scripts/tests/release-workflow.test.js:662 feeds the release step 'Error: [vitest-worker]: Timeout calling "x"\n Tests 10614 passed (10614)' and expects exit 0, so unifying on this file's onTaskUpdate-only signature reds that pin. Full reuse (release.yml calling runCli) is also blocked by sharding: release.yml:572 runs --shard=${{ matrix.shard }}/3 while the junit output path is fixed per workspace (packages/cli/vitest.config.ts:188-190, packages/core/vitest.config.ts:40-42), so the three shards overwrite one file and the surviving totals describe only the last shard.

Acceptance criterion: classify-infra-flake.test.mjs: add a fixture line Error: [vitest-worker]: Timeout calling "onCollected" beside a matching Vitest caught 1 unhandled error tally asserting tolerated: true, and a companion Timeout calling "transform" case asserting it still refuses. Removing the method-name parse reds the first; removing the harmful-method refusal reds the second. 'counts the RPC signature and vitest own unhandled tally' must keep asserting the two counts agree.

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread .github/scripts/ci/classify-infra-flake.mjs Outdated
const TESTSUITES_OPEN_PATTERN = /<testsuites\b[^>]*>/;

// eslint-disable-next-line no-control-regex -- matches the ESC that opens an SGR sequence
const ANSI_PATTERN = /\u001B\[[0-9;?]*[A-Za-z]/g;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-3: ANSI_PATTERN and LOG_TIMESTAMP_PATTERN are character-identical copies of main-failure-signature.mjs:38 and :40 — the module this file already imports extractFailingTests from — and normalizeLog re-implements the first two steps of that module's unexported cleanLine. The knowledge "an Actions log line is an RFC3339 prefix plus SGR escapes around the runner's own output" now lives in two files in the same directory with no link between them, and normalizeLog's export is unconsumed.

When the transport changes — Actions alters its timestamp format, or a CRLF/##[group] decoration starts appearing in fetched logs — the strip must be fixed twice. Both sides ARE pinned, each against its own hardcoded fixture, which is what makes the cost real rather than cosmetic: if only cleanLine is fixed, this classifier's own test still passes on its stale fixture while the anchored /^npm error path …/gm silently stops matching in production, failingWorkspaceDirs returns [], and the classifier refuses with "npm named no failing workspace". Because every branch here fails closed, the tolerance quietly stops ever firing, with no red test to say so.

Witness:

witness: not run — a duplication/ownership fact settled by reading; the nearest capability (a probe) cannot distinguish two character-identical literals any better than grep, which is what settled it: main-failure-signature.mjs:38 `const ANSI_PATTERN = /\u001B\[[0-9;?]*[A-Za-z]/g;` and :40 `const LOG_TIMESTAMP_PATTERN = /^\d{4}-\d{2}-\d{2}T[\d:.]+Z\s?/;` versus classify-infra-flake.mjs:75-76. cleanLine (main-failure-signature.mjs:47-54) is ANSI -> timestamp -> `\s+` collapse -> trim and is NOT exported; normalizeLog (classify-infra-flake.mjs:85-93) maps the first two replaces over lines. A hidden-dir-aware grep for normalizeLog returns 5 hits, all inside its own module (85, 95, 101, 112, 166), and classify-infra-flake.test.mjs's import list (4-14) omits it.

Export the two-step strip from main-failure-signature.mjs (e.g. export function stripLogDecoration(line) holding the ANSI and timestamp replaces), have cleanLine call it, and have normalizeLog map it over lines. Export the strip, not cleanLine: its extra .replace(/\s+/g, ' ').trim() is extractFailingTests's business, not the classifier's. While there, drop the unconsumed export on normalizeLog or import it in the test.

One constraint on the fix — main-failure-signature.mjs:48-54cleanLine is strip ANSI -> strip timestamp -> collapse whitespace -> trim; the shared part must stay the first two replaces only, because this file's own contract at lines 82-83 is "extractFailingTests normalizes internally too, so passing it already-cleaned text is a no-op" — the strip must remain idempotent under being applied twice.

Acceptance criterion: classify-infra-flake.test.mjs:189-199 ('normalizes ANSI escapes and Actions timestamps') asserts countRpcTimeouts, countCaughtUnhandled and failingWorkspaceDirs all see through ESC-wrapped, timestamp-prefixed lines; it goes red if the shared strip is wired in wrong. main-failure-signature.test.mjs's ANSI/extractFailingTests cases go red if cleanLine loses the collapse/trim.

— qwen3.8-max via Qwen Code /review (v0.23.0)

return refuse('no junit reader supplied');
}

const log = normalizeLog(logText);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-4: classify() normalizes the whole log and then hands the normalized text to three helpers that each normalize it again, and on the sole production input both substitutions are identity transforms — so ~5.5 full split/map/regex/join passes over the entire run's output buy nothing, and normalizeLog's idempotence becomes a load-bearing, unpinned invariant of classify()'s correctness.

ci.yml:773 is the only production caller. It passes a local tee capture, which carries no Actions RFC3339 prefix (that is added by the log API — the case normalizeLog's docblock justifies and nothing invokes) and no SGR sequences (the step sets NO_COLOR: true at ci.yml:693 and npm's stdout is a pipe, so vitest emits no colour). Measured on a capture the size the diff's own docblock reports, ~70 ms of the 156 ms classify() takes is redundant re-walking. The wall-clock cost is small against a path this file measures at up to 96m55s; the cost worth acting on is AGENTS.md's Simplicity First — the principle that document names as the one it cares about most — plus the fact that correctness now depends on an idempotence property no test pins (R1-13 shows the ANSI half of normalizeLog can be deleted with the suite staying 16/16 green).

Witness:

probe on a 2.44 MB / 24 007-line capture (the size the diff's own docblock measures):
normalizeLog is identity on an unprefixed capture: true
String#replace calls: 264077 => normalizeLog-equivalent line walks: 5.5
wall time for one classify(): 156.57 ms
5 x normalizeLog over the same log: 86.7 ms (17.3 ms each)
caller sweep: `git grep classify-infra-flake` over HEAD returns exactly one production caller (ci.yml:773) plus the test file

Normalize once in classify() and have the counting/matching internals take already-normalized text (unexported countRpcTimeoutsIn(log) etc.), keeping the exported wrappers normalizing so direct callers and the existing test still work.

One constraint on the fix — .github/scripts/ci/main-failure-signature.mjs:47function cleanLine(line) { is not exported, so the fix cannot reuse the sibling's normalizer; only extractFailingTests (line 60) is importable.

Acceptance criterion: classify-infra-flake.test.mjs 'normalizes ANSI escapes and Actions timestamps' calls countRpcTimeouts(decorated), countCaughtUnhandled(decorated) and failingWorkspaceDirs(decorated) directly on ANSI/timestamp-decorated text and asserts 1/1/['packages/cli']; it reds if normalization is deleted from the exported helpers instead of moved behind them.

— qwen3.8-max via Qwen Code /review (v0.23.0)

xml = readJunit(join(workspace, 'junit.xml'));
} catch {
return refuse(
`${workspace} wrote no junit.xml — a worker that died, not one whose RPC starved`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-5: The junit gate the whole tolerance rests on can only be satisfied by 3 of the 22 workspaces this lane runs, and for the other 19 the refusal message states a cause — a dead worker — that is false by configuration: those workspaces simply have no junit reporter.

Only packages/cli, packages/core and packages/web-shell configure a junit reporter. When a starved RPC reddens, say, packages/channels/base — a leg this classifier's own test anticipates naming — readJunit throws on every such run and stderr reads packages/channels/base wrote no junit.xml — a worker that died, not one whose RPC starved. Two costs: the exact flake class this PR set out to absorb stays red for 19 of 22 legs, and oncall is sent hunting an OOM/segfault that never happened while the real reason (a static config fact — no reporter) appears nowhere. And because the loop returns on the first missing junit, one non-junit leg timing out in the same run as cli also cancels cli's and core's clean evidence, so the tolerance fails for the legs it was written for too.

Witness:

sweep over the real population (authority: `npm query .workspace` for the workspace set, each workspace's own vitest.config.ts for the reporter):
workspaces resolved: 25; with test:ci: 22
junit-capable = 3 (packages/cli, packages/core, packages/web-shell)
NO-JUNIT = 19 (acp-bridge, audio-capture, chrome-extension, node-repl, qwen-live,
 sdk-typescript, vscode-ide-companion, channels/{base,telegram,weixin,dingtalk,
 wecom,feishu,qqbot,github,dws,gitlab}, integrations/{external-context,external-context-mem0})
probe — a packages/channels/base leg that printed a COMPLETE passing tally and died on one RPC timeout:
Vitest caught 1 unhandled error… / Error: [vitest-worker]: Timeout calling "onTaskUpdate"
 Test Files 20 passed (20) / Tests 1233 passed (1233)
release.yml gate 'Tests N passed': MATCHES    release.yml gate 'no failed tally': SATISFIED
infra-flake: not tolerated — packages/channels/base wrote no junit.xml — a worker that died, not one whose RPC starved
EXIT=1

Either put the invariant where the repo already owns repo-wide vitest-config invariants — add reporters: ['default','junit'] plus outputFile.junit to every workspace with a test:ci script and pin it per-project in scripts/tests/unit-vitest-configs.test.ts — or apply the release lane's tally fallback (.github/workflows/release.yml:586-588 accepts a run whose log carries Tests N passed and no failed tally) for a workspace with no reporter. At minimum split the reason so a workspace with no configured reporter is not described as "a worker that died".

One constraint on the fix — The file's own fail-closed rule at classify-infra-flake.mjs:17-19 ("a worker that was OOM-killed or segfaulted writes no junit and stays red"), echoed at ci.yml:766-767: a tally fallback must still refuse when the blamed workspace printed no Tests … passed line, because a killed worker prints none. Also ci.yml:817 publishes path: 'packages/*/junit.xml', which does not match nested packages/channels/<x>/junit.xml; and packages/web-shell/vitest.config.ts sets root: 'client' with outputFile.junit: '../junit.xml', so any workspace-to-junit mapping must resolve against the config's root. Note also that scripts/tests/unit-vitest-configs.test.ts's enumeration omits packages/qwen-live (pre-existing, untouched by this diff), so extending that enumeration inherits its hole.

Acceptance criterion: classify-infra-flake.test.mjs: a case blaming a non-junit workspace (e.g. packages/channels/base) whose leg printed Test Files 20 passed (20) / Tests 1233 passed (1233), asserting tolerated: true (red today), plus a companion where that leg printed no summary at all asserting tolerated: false. The existing 'fails closed when a blamed workspace wrote no junit' asserts /packages\/core wrote no junit\.xml/ and core DOES configure a reporter, so it must stay meaningful for cli/core/web-shell.

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml
npm run test:ci:workspaces -- "${retry_arg[@]}" 2>&1 | tee "$WORKSPACES_LOG"
RC=${PIPESTATUS[0]}
if [ "$RC" -ne 0 ] && [ "${QWEN_CI_TOLERATE_RPC_TIMEOUT:-1}" != '0' ]; then
node .github/scripts/ci/classify-infra-flake.mjs --log "$WORKSPACES_LOG" --root . --samples "$DISK_SAMPLES" --runner-name "${RUNNER_NAME:-}" && RC=0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-20: Forcing RC=0 makes the check green, and both existing consumers of "this red was infrastructure, not the code" gate on conclusion == 'failure' — so the ::warning:: that classify-infra-flake.mjs:54-56 calls "the only pressure to fix it (#10879)" has no automated reader, and the DFSAMPLE timeline it quotes is deleted with $RUNNER_TEMP.

A push-to-main Test job on ecs-qwen exits 1 on three onTaskUpdate timeouts with zero failing tests, so RC=0 and the conclusion is success. main-ci-failure-issue.yml:35 gates on conclusion == 'failure', so no per-commit issue is filed any more — where before this diff each occurrence produced one. qwen-autofix.yml:2677-2679 selects check_runs[] | select(.conclusion == "failure") before it fetches annotations, so the warning is never read, and its INFRA_FAILURE_SIGNATURES carries no vitest/transport pattern either, so AF-100's fleet telemetry never records the class. ci.yml writes no $GITHUB_STEP_SUMMARY anywhere, and Upload disk-pressure samples is if: failure(), so the peak-load figure the annotation quotes is not persisted. Net effect: repeated starvation on main leaves one annotation inside a run log nobody opens. Narrowing: the Linux test job is if: !cancelled() && github.event_name != 'schedule' (ci.yml:367), so the nightly arm of this scenario does not exist — the push-to-main arm is the live one, and it is the unargued sibling of the docblock's rationale, which argues the warning as pressure for the PR lanes where going green is the point.

Witness:

the real extracted ci.yml step driven with a stub npm that exits 1 on two RPC timeouts with every test passing:
::warning::Test step tolerated a runner-capacity artifact: on ecs-qwen-hk5-2, 2 vitest worker "onTaskUpdate" RPC timeout(s) with 0 failing tests (packages/cli, packages/core). Every test passed; … see #10879.
STEP EXIT=0
population sweep (grep every .yml/.mjs/.sh under .github/ for `annotations`), not just the two named gates:
 exactly ONE reader of check-run annotations repo-wide: qwen-autofix.yml:2684, reached only past
 `.check_runs[] | select(.conclusion == "failure") | select(((.output.annotations_count // 0) > 0))`
 main-ci-failure-issue.yml:35: github.event.workflow_run.conclusion == 'failure' && head_branch == 'main' && …
 INFRA_FAILURE_SIGNATURES (qwen-autofix.yml:130): no vitest/transport pattern
 grep -c GITHUB_STEP_SUMMARY .github/workflows/ci.yml -> 0
 'Upload disk-pressure samples' (ci.yml:788-789): if: '${{ failure() }}'
comparative half off the diff's own `-` lines: at base the step was `npm run test:ci:workspaces …; RC=$?` with no tolerance, so a zero-failing-test red on main reached conclusion == 'failure' and main-ci-failure-issue.yml:220-226 files unconditionally once analyze runs

Give the verdict a reader, or restate the claim. Cheapest durable record: write warningLine(verdict) to $GITHUB_STEP_SUMMARY in the tolerated branch and keep the annotation. If the capacity case needs counts, make one of the existing consumers see it — e.g. add the RPC signature to INFRA_FAILURE_SIGNATURES so AF-100 records the class — or edit classify-infra-flake.mjs:54-56 to say the auto-filed main-CI issue is intentionally given up for tolerated runs.

One constraint on the fix — main-ci-failure-issue.yml:35github.event.workflow_run.conclusion == 'failure'. A durable record that depends on that filer would have to keep the run red, which defeats the tolerance; the record must be written from inside the green run.

Acceptance criterion: classify-infra-flake.test.mjs 'runCli exits 0 on a tolerated verdict and 1 otherwise' already asserts "a tolerated verdict must be announced, never silent" against captured stdout; extend that assertion to the new channel so deleting the step-summary write reds the test. A comment-only fix has no witness (N/A).

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread .github/workflows/ci.yml
node .github/scripts/ci/classify-infra-flake.mjs --log "$WORKSPACES_LOG" --root . --samples "$DISK_SAMPLES" --runner-name "${RUNNER_NAME:-}" && RC=0
fi
if [ "$RC" -eq 0 ]; then
npm run test:scripts -- "${retry_arg[@]}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-21: The classifier guards only the workspaces leg. The npm run test:scripts leg the same step runs next executes the same vitest on the same starved host, is not tee'd, and is not classified — and its exit code becomes the step's, so the Test job can still go red on a zero-failure run.

test:scripts is vitest run --config ./scripts/tests/vitest.config.ts (root package.json:52, measured at 2144 tests) and scripts/tests/vitest.config.ts:56 carries dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux', i.e. fatal on this Linux lane. On a host at load 270 the worker RPC starves there exactly as it does in the workspaces leg, so a run where cli and core were tolerated at line 773 and the scripts leg then exits 1 on one onTaskUpdate timeout produces the same false red the PR set out to remove — with nothing captured to classify afterwards, since only the workspaces command is tee'd. The added workflow comment ("classify the exit against the junit totals instead of reporting a failure no test caused") reads as covering the step, so nothing tells the next reader it covers half of it.

Witness:

ARM A (workspaces leg green; scripts leg prints the identical RPC signature, exits 1):
 STEP_EXIT=1  test:scripts leg ran: 1  classifier invoked: 0
ARM B (workspaces leg tolerated; scripts leg then dies on the same signature):
 STEP_EXIT=1  ::warning::Test step tolerated … (packages/cli) …  classifier invoked: 1 (workspaces leg only)
the extracted step tees exactly one log:
 WORKSPACES_LOG="${TMPDIR}/test-ci-workspaces.log"   (extracted line 64)
 … | tee "$WORKSPACES_LOG"                           (extracted line 66)
exposure corroborated from the repo itself: scripts/tests/vitest.config.ts:56 `dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux'` under the comment 'RPC-timeout exemption; see scripts/tests/unit-vitest-configs.test.ts', whose configs map includes 'scripts/tests' as one of the exposed projects — 78 test files, same step, same host

Either tee and classify the scripts leg too, or say in the added comment that the tolerance covers the workspaces half only and why:

npm run test:scripts -- "${retry_arg[@]}" 2>&1 | tee "$SCRIPTS_LOG"
RC=${PIPESTATUS[0]} TEE_RC=${PIPESTATUS[1]}
if [ "$RC" -ne 0 ] && [ "$TEE_RC" -eq 0 ]; thenfi

Note this leg writes no junit at all and npm blames the repo root (which NPM_ERROR_PATH_PATTERN does not match), so classifying it needs the tally fallback from R1-8.

One constraint on the fix — no-ak-integration-ci.test.js:104-125 requires, in all three legs and in order, set +e -> npm run test:ci -> RC=$? -> set -e -> pkill -TERM -P "$SAMPLER_PID" … -> kill "$SAMPLER_PID" … -> exit "$RC", and package-scripts.test.js:117-118 require the literals npm run test:ci:workspaces -- "${retry_arg[@]}" and npm run test:scripts -- "${retry_arg[@]}" to remain substrings of the step.

Acceptance criterion: scripts/tests/ci-platform-lanes.test.js already asserts properties of this step's run block (lines 645-651 pin --test-concurrency=1 on every HELPER_TESTS invocation); add an assertion there that the classifier is invoked for the scripts leg too, so removing the second guard fails the scripts suite. package-scripts.test.js:118's toContain('npm run test:scripts -- "${retry_arg[@]}"') survives a tee because it is a substring match, so it is not the witness.

— qwen3.8-max via Qwen Code /review (v0.23.0)

yiliang114 and others added 2 commits September 6, 2026 06:34
Two ways the infra-flake classifier could certify a run it never
checked. `npm run test:ci:workspaces` runs the integrations/* workspaces
too, but the blame-line pattern captured only paths under packages/, so
a workspace npm blamed under integrations/ was neither junit-checked nor
refused and rode another leg's RPC timeout to a green required check.
And a workspace whose vitest include matched nothing writes a
well-formed report with tests="0", which read as "zero failures" — no
test failed and no test ran are different claims.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtow5pgslz
The classifier's only evidence is the tee capture, so a sink that hit a
write error on the job TMPDIR — the ENOSPC this same step documents —
handed it a prefix of the run, and every "no FAIL line" gate then read
text that was never captured. Read both pipeline statuses in one
statement, since PIPESTATUS is reset by anything in between, and refuse
to classify when tee itself failed.

The kill switch also compared != '0', so an operator reaching for 'off'
during an incident, or a trailing space, left the tolerance silently on.
Only '1' enables it now; the || '1' default in the binding keeps today's
behaviour when the repository variable is unset.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtow5pgslz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants