fix(ci): reduce ENOSPC and load-sensitive test flakes - #8982
Conversation
The test spawned three full agent subprocesses (~10s of wall time) to
pin a parse guard that is read once at module load. Under load spikes
on the shared self-hosted runner pool, one of those spawns failed or
stalled, failing CI at random (e.g. run 31556596385); the assertion
also discarded the script's failure.md, hiding the cause.
Collapse it to one run with the nastiest value (-1) — any armed
instant-true window kills the healthy stub agent at the first idle
tick — and assert on {status, failure} so a future flake names its
cause. Add retry: 2 to the script-test vitest config for the
remaining load-sensitive spawn tests; a real regression fails every
attempt.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the iteration — this pass re-reviews the full deflake package on Template looks good ✓ — all sections present, including the restored Environment section. Problem: observed, and now three independent failure classes, each with evidence:
Direction: aligned — pure CI/test-infrastructure deflake with no product surface, and it now lands the split @qqqys asked for: the inert Size: zero production logic lines — 9 test files, Approach: coherent — every added piece maps to an observed failure mode rather than speculative hardening. The Risk: no elevated risk signals — no changed file matches the high-risk path patterns. Moving on to code review. 🔍 中文说明感谢持续迭代——本轮在 模板完整 ✓——各节齐全，包括恢复的 Environment 小节。 问题：已观测到，且现在是三个相互独立的故障类别，各有证据：
方向：对齐——纯 CI/测试基建 deflake，不涉及产品面，并且现在落实了 @qqqys 要求的拆分：无效的 规模：生产逻辑行数为零——9 个测试文件、 方案：自洽——每处新增都对应一个已观测的故障模式，而非理论加固。 风险：无升级风险信号——没有改动文件命中高风险路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal for this PR's four problems — written from the title and "Why it's needed" before reading the diff — was: shrink the watchdog test's spawn count and make its assertions diagnostic, with retry confined to the suite that spawns real processes; route Linux test temp dirs to a real short disk-backed path and sample the filesystem during the run while preserving the original exit code; tear the large fixtures down per-test; and cut the unbounded review-context expansion rather than raising the 128 cap. The PR matches that proposal point for point, and the review history shows it converged there honestly — the inert Verified the load-bearing claims against the code (worktree reads, base tree + diff):
No critical blockers, no weakened or deleted checks, no convention violations. TestingUnattended CI run — triage never executes PR code; the evidence below is the PR's own CI on the reviewed commit, fetched via the API, plus API-verified lineage. On Lineage, verified via API rather than taken on the author's word: the preceding head The author's local numbers (watchdog 1/1, repository-context 60/60, socket-sensitive files 78/78, stale-bundle 28/28, committed-policy 5/5, provider 34/34) are the author's claims, not evidence — the pending Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 There is no sandboxed lane to name here: this is test-infrastructure deflake with no product-behavior claim for 中文说明代码审查在看 diff 之前，我只凭标题和"Why it's needed"写下的独立方案是：压缩 watchdog 测试的 spawn 数量并让断言自带诊断、retry 仅限于真正启动进程的套件；把 Linux 测试临时目录路由到真实的短路径磁盘目录并在运行期间采样文件系统、同时保留原始退出码；按测试清理大型夹具；砍掉 review-context 的无界扩展而不是抬高 128 上限。PR 与这个方案逐点吻合——而且审查历史表明它是诚实地收敛到这里的：无效的 对照代码核实了关键论断（worktree 读取，基线树 + diff）：
无关键阻塞项，无削弱或删除的检查，无约定违规。 测试无人值守 CI 运行——triage 从不执行 PR 代码；以下证据是被审 commit 上 PR 自己的 CI（经 API 获取），以及经 API 核实的谱系。 在 谱系（经 API 核实，不是照抄作者说法）：前一 head 作者的本地数字（watchdog 1/1、repository-context 60/60、socket 敏感文件 78/78、stale-bundle 28/28、committed-policy 5/5、provider 34/34）是作者声明，不是证据——该 commit 上待完成的 这里没有可指名的沙箱验证通道：这是测试基建 deflake，没有供 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a clean, well-iterated deflake with every prior review finding verified addressed at this head; approval is deferred only because the reviewed commit's own CI is still in flight. Stepping back: this is what a deflake PR should look like after review pressure, not despite it. My independent proposal before reading the diff was the same four-part shape the PR delivers, and the history shows it got there by accepting hard feedback: the inert On the two standing change requests: both target code that no longer exists or has been superseded at this head. The Verdict: approve — deferred until CI lands green. The three checks that matter here ( 中文说明置信度：4/5 —— 一个干净、经过充分迭代的 deflake；此前所有评审发现在当前 head 上均已核实解决；批准推迟的唯一原因是被审 commit 自己的 CI 仍在运行。 退一步看：这正是 deflake PR 在评审压力之下（而非无视压力）该有的样子。我在读 diff 之前的独立方案与 PR 交付的四段结构相同，而历史表明它是靠接受尖锐反馈到达这里的：无效的 关于两个仍然挂起的 changes request：它们针对的代码在当前 head 上已不存在或已被取代。@doudouOUC 那条背后的 结论：批准——但推迟到 CI 变绿。与本 diff 相关的三个检查（ — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): This PR deflakes the idle-watchdog script test: it collap...: did not run build/lint/typecheck (brief lists build-lint) — node_modules is not installed in this worktree; the changed lines are test code, comments, and a s….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：This PR deflakes the idle-watchdog script test: it collap...：did not run build/lint/typecheck (brief lists build-lint) — node_modules is not installed in this worktree; the changed lines are test code, comments, and a s…。
— qwen3.8-max via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: None — I completed all checks within the budget..
中文说明
已审查。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：None — I completed all checks within the budget.。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
test:ci runs every workspace in parallel, and the cli/core suites each pinned a fixed 8-16 thread pool regardless of the machine — on a 4-core hosted runner that is an 8x oversubscription before any neighbor job exists, and the shared self-hosted hosts run several registrations at once. The contention is what blows the 15s ceilings those configs already complain about: tests that pass locally in milliseconds time out, and vitest workers lose their RPC under the pile. Size the pools to os.cpus() (capped at the old fixed values so large machines lose nothing) and add retry: 2 for the residual load spikes a real regression still fails through every attempt of.
…nto fix/ci-idle-parse-guard-flake
|
Scope update — this PR now carries the full deflake package, on top of the latest main (including the #8984 bridge-test fix):
Verified locally: configs load, prettier/eslint clean, smoke suites pass; the script suite passes 162/162 with mutation-tested sensitivity on the rewritten case. |
yiliang114
changed the title
…ents Review feedback: the single -1 run could not catch a > 0 → >= 0 boundary edit (0 is the operator's documented disable sentinel and would arm a zero-length window), and the comments overstated what the run and the retry guarantee. Probe-verified: a >= 0 mutant now fails the idleMs: 0 arm with the value named in the assertion diff. Two short runs still halve the old three-run spawn volume. Reword the retry comments to claim only deterministic regressions fail every attempt.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 79 passed · 0 failed · 79 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：79 通过 · 0 失败 · 79 总计 Verification reportPR #8982 verification — fix(ci): deflake load-sensitive script tests and stop test pools from oversubscribing CIVerdict: 中文摘要
Central claim + mutation A/BCentral claim: the deflaked env-parse test (3 agent spawns → 2) still pins every regression class of the Harness:
No mutant regressed from killed to survived. The kill sets are identical (4/9 each side); the dropped NaN arm killed nothing on base — empirical confirmation of the PR's claim that "no run can pin the non-numeric class". Positive controls: the four kills prove the harness can go red; wt is green on both sides; every killed cell shows The killed Timing A/B (same harness, vitest's own
3.1× faster; spawn volume 3 → 2 per run of the test. (See Finding F2 re the "~2s" claim.) Retry mechanism A/B ( Pool sizing (scripted evaluation of the exact expressions for N=1..64, 23/23): min ≤ max everywhere; capped at the old fixed values (≤8/≤16); N≥16 identical to old (8/16, "large machines lose nothing"); the 4-core hosted runner the PR names goes 8/16 → 2/8. On this 64-core box the pool evaluates to 8/16, i.e. unchanged — the changed values only bite on ≤15-core hosts, which is where the oversubscription was. Targeted gates ( Findings (non-blocking)F1 (nit, comment accuracy). The new test comment says the non-numeric class "rests on the F2 (nit, description). "~2s instead of ~10-14s": measured 3.12s tests-duration (4.2s wall) vs 9.76s (10.5s wall). Direction and order-of-magnitude correct; the "~2s" figure understates by ~1s. F3 (pre-existing coverage gaps, completeness reporting — not merge conditions). Not covered
MethodologyEnvironment: the CI verify container (node 22, 64 cores), Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8982 fix(ci): deflake load-sensitive script tests and stop test pools from oversubscribing CI
Author: @yiliang114 | Type: Infra/CI + Test
HEAD reviewed: d94ea47 (base 1570e6c, 6 commits) | Size: +51/-12 across 4 files
CI at HEAD: FAILING - Test (ubuntu-latest, Node 22.x) (run 31578748037)
Findings Summary
- Critical: 0
- Major: 3
- Minor: 3
- Nit: 1
Key Observations
The script-test half is solid. I verified the -1 / 0 two-probe shape against .qwen/skills/autofix/scripts/run-agent.mjs:412-419 (idleTick = Math.max(250, Math.min(30_000, floor(window/4))), kill on Date.now() - lastOutputAt >= window): with a window of -1 or 0 the tick collapses to 250 ms and the healthy stub is killed well inside its 1.5 s life, so both probes really do bite. The NaN reasoning in the new comment is also correct - elapsed >= NaN is permanently false, so no healthy-agent run can ever pin that class. The {idleMs, status, failure} assertion is a real diagnostic improvement, and retry: 2 is confirmed live in the CI log (each failed test prints 3 attempts).
The load-shaping half does not do what its comments claim, on two independently verified counts:
1. poolOptions.threads is inert in these two configs. The repo is on vitest ^3.2.4, whose default pool is forks (v3 docs). Neither packages/cli/vitest.config.ts nor packages/core/vitest.config.ts sets pool: 'threads', so minThreads/maxThreads are never read. The live knob is poolOptions.forks.maxForks, which already defaults to available CPUs. integration-tests/vitest.config.ts:30 in this same repo gets this right (pool: 'forks' + poolOptions.forks.{min,max}Forks); packages/sdk-typescript/vitest.config.ts:33-36 has the same latent problem.
2. npm run test:ci --workspaces --if-present --parallel does not run workspaces in parallel. --parallel is not an npm run-script config. Reproduced locally on npm 11.12.1 with a 2-workspace fixture: npm warn Unknown cli config "--parallel" and workspace B started only after A finished. Confirmed again in this PR's own CI log: channel-gitlab reports Duration 1.05s at 08:48:18.30 and external-context starts at 08:48:18.43 - strictly sequential. So the "a fixed 16-thread pool per package oversubscribes a 4-core hosted runner 8x over before a single neighbor job exists" arithmetic (repeated in the scope-update comment) does not hold: only one package's pool is alive at any instant.
Consequently the formula's motivation is doubly unsupported, and maxThreads = Math.min(16, Math.max(4, os.cpus().length * 2)) returns exactly the old 16 on any host with >= 8 cores - including the shared self-hosted hosts the comment blames, because os.cpus().length reports the whole host, not this runner's share.
Merge Readiness
mergeable_state: blocked.Test (ubuntu-latest, Node 22.x)is red at HEAD: 11 failed suites / 57 failed tests inpackages/cli, headlined byError: ENOSPC: no space left on device, open '/tmp/n4jYk0_fCROErd8qkw5jP/web/...'- the runner-oversubscription problem surfacing as disk exhaustion, which pool sizing cannot fix.test:ciisnpm run test:ci --workspaces ... && npm run test:scripts. The workspace leg exited 1, sonpm run test:scriptsnever ran in that job: the twoscripts/tests/files this PR actually changes were not exercised by the failing run.- Same run:
src/ui/utils/clipboardUtils.test.ts (32 tests) 91484ms, with individual tests at 8 s. Good independent evidence of the contention this PR targets - and evidence that the current pool settings are not shaping it.
Cross-Validation
Both existing reviews were submitted at older commits (qwen-code-ci-bot at edfcadb, 06:06Z; doudouOUC at 13a61cb, 07:40Z) - i.e. before the pool-shaping commit 21c00e1 (08:01Z). Neither reviewer has seen packages/cli|core/vitest.config.ts at all, which explains the gap below.
| Finding | Other reviewer | My assessment (verified against HEAD d94ea47) |
|---|---|---|
R1-1 Collapsing to a single -1 run drops the 0 rejection class; comment over-claims NaN |
qwen-code-ci-bot + doudouOUC | Confirmed valid - and already fixed at HEAD. qwen-autofix-workflow.test.js:12315 is now for (const idleMs of [-1, 0]) and the comment now explicitly states the non-numeric class cannot be pinned by a run. Both inline comments are OBSOLETE and can be resolved. |
| R1-2 "a real regression fails every attempt" over-claims for intermittent regressions | qwen-code-ci-bot (scripts/tests/vitest.config.ts:36) |
Confirmed valid - and already applied at HEAD. All three configs now read "a real deterministic regression". OBSOLETE. I extend it below (C4): the residual risk is not the wording but that a retried-then-passed test is reported as a clean pass to the junit reporter. |
C1 poolOptions.threads is dead config - vitest 3 defaults to pool: 'forks' |
- | Unique-Mine (Major) - packages/core/vitest.config.ts:51-52, packages/cli/vitest.config.ts:183-184 |
C2 --parallel is not an npm flag; workspaces run serially |
- | Unique-Mine (Major) - packages/core/vitest.config.ts:46; empirically disproven locally and in this PR's CI log |
C3 os.cpus().length measures the host, not the runner's share; >= 8 cores yields the old 16 |
- | Unique-Mine (Major) - packages/cli/vitest.config.ts:184 |
C4 retry: 2 silently widened to all of cli+core; PR body still says "the script-test vitest config" |
- | Unique-Mine (Minor) - packages/cli/vitest.config.ts:152 |
C5 scripts/tests/vitest.config.ts still hardcodes minThreads: 8 / maxThreads: 16 |
- | Unique-Mine (Minor) - the heaviest suite in the repo was left out of the resizing |
| C6 Test title still advertises the non-numeric class it no longer exercises | - | Unique-Mine (Minor) - the bots flagged the comment over-claim (fixed); the it() title still over-claims |
| C7 Shorter stub narrows the starvation margin for kill detection | - | Unique-Mine (Nit) |
| Disagreement with the author's scope-update comment | @yiliang114 | I disagree with the stated premise ("test:ci runs every workspace in parallel", "8x oversubscription on a 4-core hosted runner"). Both halves are empirically false - see C1/C2. The contention is real; the mechanism named for it is not. |
Additional Audit Coverage
Areas I checked independently that came back clean - recorded so the negative results are on file:
- Watchdog kill-path math: traced
run-agent.mjs:33-40(Number.isFinite(p) && p > 0) and:412-419; both new probes provably arm a 250 ms tick. Correct. - Retry idempotency of the modified test:
runAgentusesmkdtempSync+finally { rmSync(..., {recursive:true, force:true}) }, so a vitest retry re-spawns into a fresh tempdir with no carry-over. Safe underretry: 2. minThreads > maxThreadsinvariant: evaluated the formula forcpus= 1..64;minis capped at 8 andmaxfloored at 4, and the pair never inverts. No config error.- Sibling config survey:
packages/sdk-typescript/vitest.config.ts:31-36(retry 2 + threads 2-4, same inert-pool issue) andintegration-tests/vitest.config.ts:28-34(retry 2 + correctpool: 'forks').retry: 2therefore follows existing precedent - it is the blast radius (C4), not the mechanism, that I flag. - AGENTS.md compliance: test-infra only, nothing under
packages/core/src/**, so the maintainer-only core gate does not apply. The "comments only when the why is non-obvious" rule is respected in form - the problem is that two of the whys are factually wrong, which is worse than no comment because the next maintainer will tune against them. - Consumer impact of the config changes:
packages/{cli,core}/package.jsonboth definetest:cias barevitest run; no CLI flag orVITEST_*env inci.ymloverridespool/poolOptions, and.npmrccarries only a registry line. Nothing else reads these knobs.
Industry Context
- Retry-on-flake is mainstream (Bazel
--flaky_test_attempts,pytest-rerunfailures,jest.retryTimes), but the near-universal companion practice is to surface the retry rather than let it vanish: Bazel marks the targetFLAKY, pytest prints a rerun summary. Here,packages/coreemitsjunitand CI publishes it, so a test that passes on attempt 3 is indistinguishable from a clean pass - and the repo's ownqwen-ci-flaky-rerun.ymlpatrol, which triages failed runs, loses its input. Keepingretry: 2is fine; adding flake surfacing (e.g.onTestRetry/a reporter that annotates retries, or--reporter=jsonpost-processing) preserves the signal. - CPU sizing: current Node guidance is
os.availableParallelism()(Node >= 18.14; this repo mandates >= 22) overos.cpus().length, because the former honours the CPU affinity mask. Neither honours cgroup CPU quota (libuv#4146), which is exactly why CI systems normally pin the value explicitly - vitest natively supportsVITEST_MAX_THREADS/VITEST_MAX_FORKSfor this.
Final Verdict
Needs changes (load-shaping half only). The script-test deflake is well-reasoned, correctly targeted and ready as-is; I would merge it on its own today. The packages/{cli,core} pool resizing should either be corrected (pool-agnostic maxWorkers, or pool: 'threads', or poolOptions.forks) with the two factual premises removed from its comments, or split out of this PR so the verified half is not held hostage to the unverified one. Note also that per CONTRIBUTING.md #2 this is now two changes in one PR, and the description still only describes the first.
中文说明
结论：脚本测试部分可以合；线程池改造部分需要修正。
- 线程池配置是无效的：仓库使用
vitest ^3.2.4，其pool默认为forks。packages/cli和packages/core的配置都没有设置pool: 'threads'，因此minThreads/maxThreads根本不会被读取。同仓库的integration-tests/vitest.config.ts:30写法才是对的。 - 注释里的前提不成立：
npm run test:ci --workspaces --parallel并不会并行执行。npm 根本没有--parallel这个配置（本地 npm 11.12.1 复现：警告Unknown cli config "--parallel"，两个 workspace 串行执行）；本 PR 自己的 CI 日志也证实了串行（gitlab 包08:48:18.30结束，external-context08:48:18.43才开始）。所以“4 核 runner 上 8 倍超卖”的算法不成立。 os.cpus().length测的是整台机器，不是本 runner 的份额；只要宿主机 >= 8 核，maxThreads仍然等于旧值 16——而那正是注释所指责的自托管共享主机场景。- 已有两位审查者的意见已在 HEAD 修复（
0探针已恢复、“deterministic”措辞已采纳），它们的行内评论现已过时，可以关闭。两位审查者都是在线程池 commit 之前审查的，因此未覆盖上述 1-3 项。 - 当前 CI 是红的（
/tmpENOSPC + cli 57 个用例失败），且因为&&短路，npm run test:scripts未执行——本 PR 真正改动的两个脚本测试文件在这次 CI 中根本没跑到。
This review was generated by QoderWork AI
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
The idle-watchdog test simplification is sound, including the restored 0 boundary probe, but the worker-pool half of this PR is not active at runtime. I am requesting changes for the inline blocker.
I also inspected the current Ubuntu failure: the target idle-timeout test passed; the job later failed because the self-hosted runner exhausted /tmp (ENOSPC), causing 11 CLI suites / 57 tests to fail. That environmental failure is separate from the test change, although retry: 2 repeated each persistent ENOSPC failure three times and cannot recover this class.
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
The poolOptions.threads changes had no effect because vitest 3 defaults to pool: 'forks', making minThreads/maxThreads inert. Revert to the original fixed 8-16. The script-test deflake (idle-watchdog improvements, retry: 2) is kept. Add a source-text pin for the Number.isFinite guard in run-agent.mjs, covering the non-numeric class that subprocess-based tests cannot exercise.
… inode hold ENOSPC failed the Test step mid-suite on two different self-hosted machines (actions-runner-test-22, actions-runner-test-11) in ~20s bursts — 132 of 147 errors were mkdtemp failures — while the hosts look healthy afterwards, so a post-mortem df finds nothing. Two changes: 1. Sample /tmp space and inodes every 10s during the test step and dump the full df state when it fails, so the next occurrence records whether inodes or a tmpfs cap is what exhausts. 2. The manifest-repository-context suite held every 16k-file fixture tree until afterAll (~164k live inodes for the whole file); tear down per test instead so at most one tree is live at a time, removing the suite's own spike contribution either way.
…class A healthy-agent run can never pin the non-numeric rejection class: a NaN window never satisfies the >= kill comparison, so no run shape fails on it. Pin the guard expression itself in the runner source instead (this file's existing source-text pin style), and rename the test to claim only the non-positive classes the runs actually pin.
|
The failure on The all-rules case now resolves 86 paths. The committed-policy suite passes 5/5, the provider suite passes 34/34, and Prettier passes. New CI is running on |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 86 passed · 0 failed · 86 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：86 通过 · 0 失败 · 86 总计 Verification reportPR #8982 verification (round 2) — fix(ci): reduce ENOSPC and load-sensitive test flakesVerdict: 中文摘要
Previous-finding status (round 1 → round 2)Round 1 verified head
Central claim + A/BCentral claim: removing Harness
Union arithmetic measured independently: head widest (86) + skills census (43, via Suite-level A/B flip (witness 16/16 scripted assertions in the harness; vacuity crossover V1 additionally proves the head committed test fails against the base manifest naming the Delta surfaces verifiedci.yml test-step wrapper (3 byte-identical blocks, 1479 chars each, in
This replay is uncalibrated — no real production step artifact is fetchable in-sandbox (no token), so the cells prove the block's mechanics, not byte fidelity to a past run; see Not covered. Witness Idle-watchdog deflake (carried over, input closure changed). The test shape changed since round 1 (renamed, 2 arms, source-text pin added), so the mutation matrix was re-run, not diffed.
No mutant regressed killed → survived; the head kill set strictly contains the base kill set. Adjudication cell: head shape with the pin lines removed × Retry mechanism ( Teardown/cleanup hardening. Verified through the gates below (per-test fixture teardown in the manifest suite, reconciler-stop tracking + Gates (scripted verdicts, head = merge tree unless noted)
Findings (non-blocking)F-new-1 (nit, description counts). "the committed-policy suite passes 5/5, and the provider suite passes 34/34" — 5/5 confirmed; no suite measures 34: F1 (carried, nit). Comment attribution of NaN rejection to F3 (carried, completeness). Environment discrepancy (not a PR finding; for the record). The main checkout's Not covered
MethodologyEnvironment: the CI verify container (node v22.23.2, 64 cores, Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not explored to full depth (tool budget reached): You are review agent 3a — Agent 3a: Reuse & duplication.: 无。; You are review agent 6c — Agent 6c: Undirected audit — ...: None. I examined all 8 files in the diff, the referenced source files ( ls.ts , run-qwen-serve.ts , workflow-helpers.js , paths.ts ), and verified the macOS ….
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未探索到全部深度（达到工具调用预算）：You are review agent 3a — Agent 3a: Reuse & duplication.：无。；You are review agent 6c — Agent 6c: Undirected audit — ...：None. I examined all 8 files in the diff, the referenced source files ( ls.ts , run-qwen-serve.ts , workflow-helpers.js , paths.ts ), and verified the macOS …。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
|
Resolved the remaining review suggestions at head a67c0bf as intentionally not changed: they were extraction/test-hardening/future-parallelism suggestions outside this CI flake fix, with no current PR-caused failure. No code changed in this closeout. Current automatic review is pending. |
Superseded by later commits: the inert pool-sizing change and blanket package retries were removed, and Linux test temp files now use a job-private real /var/tmp directory. The current head has no unresolved thread from this review.
|
Added I also stress-checked the same-session load/reload handshake already in this PR. At the exact base, 7 of 60 focused rounds failed with the CI signature (4 load, 3 reload); at the fixed head, all 60 rounds / 120 cases passed. No retry or timeout increase was added for either race. |
|
Follow-up for the remaining load-sensitive failure: job 94211113721 timed out in the |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI at this commit; the changed suites ran on Linux only (locally and in the ubuntu Test job).
Not explored to full depth (tool budget reached): "PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…": did not run the changed vitest suites in this worktree (dependencies not installed here) — dynamic behavior of the rewritten sandbox test and waitForImageInges…; "PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…": could not verify from here that the ECS fleet's RUNNER_NAME values actually start with ecs-qwen- (the maxWorkers gate); CI runs on those hosts are the ora…; "PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…": did not re-run the new no-ak workflow test itself; I replicated its bash execution manually instead.; "PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…": did not execute the rewritten workflow-sandbox.test.ts or server.test.ts suites (node_modules not installed in this review worktree; npm ci + run exceeded…; "PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…": none — all planned checks are complete. I've used about 15 calls., and 4 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI at this commit; the changed suites ran on Linux only (locally and in the ubuntu Test job)。
未探索到全部深度（达到工具调用预算）："PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…"：did not run the changed vitest suites in this worktree (dependencies not installed here) — dynamic behavior of the rewritten sandbox test and waitForImageInges…；"PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…"：could not verify from here that the ECS fleet's RUNNER_NAME values actually start with ecs-qwen- (the maxWorkers gate); CI runs on those hosts are the ora…；"PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…"：did not re-run the new no-ak workflow test itself; I replicated its bash execution manually instead.；"PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…"：did not execute the rewritten workflow-sandbox.test.ts or server.test.ts suites (node_modules not installed in this review worktree; npm ci + run exceeded…；"PR #8982 (QwenLM/qwen-code) is a CI/test deflake: it…"：none — all planned checks are complete. I've used about 15 calls.，另有 4 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
Fixed the temp cleanup trap so cleanup failures cannot flip a successful wrapped test step to failed, and pinned it in the existing CI wiring test. Verified with the focused no-AK integration CI test, Prettier, and diff check. The remaining suggestions were left out as stale cleanup policy or mutation-only test hardening. CI/review are pending on the new head. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "PR #8982 is a CI/test deflake package: it makes the…": none — all checks above completed within budget.; "PR #8982 is a CI/test deflake package: it makes the…": none — all checks I started completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget.; "PR #8982 is a CI/test deflake package: it makes the…": none — all planned checks completed (~22 of ~42 calls).; "PR #8982 is a CI/test deflake package: it makes the…": none — all planned checks completed within budget., and 1 more.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未发现问题。LGTM！✅
未探索到全部深度（达到工具调用预算）："PR #8982 is a CI/test deflake package: it makes the…"：none — all checks above completed within budget.；"PR #8982 is a CI/test deflake package: it makes the…"：none — all checks I started completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.；"PR #8982 is a CI/test deflake package: it makes the…"：none — all planned checks completed (~22 of ~42 calls).；"PR #8982 is a CI/test deflake package: it makes the…"：none — all planned checks completed within budget.，另有 1 条。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.11)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Approve — the pool diagnosis is the real fix here, and it holds up
I independently re-derived the central claim rather than taking the description's word for it, and it is exactly right. Resolving each config through vitest's own createVitest on a 12-core host:
| config | resolved pool |
resolved maxWorkers |
poolOptions.threads |
|---|---|---|---|
base 4a281f2e, RUNNER_NAME=ecs-qwen-linux-01 |
forks |
(unset — default) | {minThreads: 8, maxThreads: 16} |
PR head 254758b1, RUNNER_NAME=ecs-qwen-linux-01 |
forks |
3 | {} |
PR head 254758b1, RUNNER_NAME=github-hosted-ubuntu |
forks |
(unset — default) | {} |
So the base really did carry maxThreads: 16 while resolving maxWorkers to the default under the forks pool — the limit was dead config, exactly as described, and every suite could fan out to availableParallelism - 1 forks on a host carrying several runner registrations. maxWorkers: '25%' lands the intended 3-of-12 on ECS and leaves hosted/local untouched. maxWorkers?: number | string in the vitest 3.2.4 types confirms the percentage form is supported rather than silently ignored.
I also verified the exit-status chain by running the wrapper's shape with a stub in place of npm run test:ci: stub exit 0 → wrapper 0, stub 1 → wrapper 1, stub 7 → wrapper 7, with the failure dump emitted only on the nonzero paths. set +e / RC=$? / set -e is the correct pattern here given the workflow-level bash default runs with errexit, and every command after the restore carries || true, so nothing in the teardown can mask $RC.
Suites I ran at the PR head: shell-ast-parser-lazy + workflow-sandbox + ls → 183/183, and scripts/tests/no-ak-integration-ci.test.js → 9 passed / 1 skipped. manifest-repository-context.test.ts and .committed.test.ts pass except for the keeps the manifest un-ignored and tracked case, which fails only in my scratch export — it is skipIf(!inGitWorktree()) and my git archive tree sits nested inside another git worktree, so the guard sees a repo whose ignore rules do not match. Not a PR issue.
Dropping the skills relatedPaths from .qwen/review-context.json is the one genuinely behavioral change, and I think the trade is right: the alternative was raising a wire bound to accommodate unbounded growth in sibling skill files, and the core-skills domain plus the diff itself still carry the signal. Worth knowing it bites reviews of SKILL.md changes specifically, which is a common shape in this repo lately.
Three non-blocking notes, none of which I would hold the PR for:
trap 'rm -rf "$TMPDIR" 2>/dev/null || true' EXITis single-quoted, so$TMPDIRresolves at trap time rather than registration time. It is correct today because nothing reassignsTMPDIRafter the trap, butrm -rfon a late-bound variable is a sharp edge for whoever edits this block next —"$QWEN_CI_TMPDIR"would be inert against that. Slightly awkwardly,no-ak-integration-ci.test.jsnow asserts the literal"$TMPDIR"spelling, so hardening it means updating the assertion too.preserves test failures in every wrapped OS jobpins command text and source order viaindexOf, not behavior. It would stay green through a change that keeps the ordering but breaks the semantics — the exit-code chain I measured by hand is the thing actually worth locking, and it is not what the test locks.export TMPDIR="${RUNNER_TEMP:-${TMPDIR:-/tmp}}"runs on all three OS jobs, but Node on Windows readsTEMP/TMPand ignoresTMPDIR, so that line is inert there. Harmless, just not load-bearing outside Linux/macOS.
On coverage: test_macos and test_windows are if: github.event_name == 'merge_group', so both skipped on this PR and the new wrapper's non-Linux paths get their first real exercise in the merge queue — a Git Bash surprise (e.g. pkill absent, though it is || true-guarded) would surface as a queue failure rather than here. That is the repo's existing gating, not something this PR changed; flagging it only so the merge-queue run gets a look rather than a rubber stamp. Test (ubuntu-latest) passed in 18m50s, which is the path this PR most directly targets.
6 participants
What this PR does
This PR makes the required Test gate less sensitive to shared-runner load and
/tmppressure without retrying failed tests./var/tmp, then samples that filesystem's space and inodes plus available memory every 10 seconds while preserving the original test exit status. Real short paths keep Unix sockets below Linux's 108-byte limit without breaking tests that intentionally compare resolved paths.ecs-qwen-*runners. This replaces a threads-only limit that never applied because Vitest actually uses the default forks pool; GitHub-hosted and local runs keep their existing concurrency.core-skillsdomain signal, so adding bundled skills cannot overflow the 128-file wire bound.Why it's needed
Run
31556596385failed on the idle-watchdog guard test under shared-runner load and then passed on retry. Separately, Test jobs on two self-hosted machines failed in roughly 20-second ENOSPC bursts: 132 of 147 errors were temporary-directory creation failures even though the hosts looked healthy afterwards.The repository-context suite contributed a large avoidable inode spike by retaining ten 16k-entry fixture trees at once. Releasing each tree after its test removes that contribution. Routing temporary files out of shared
/tmpavoids the observed tmpfs/shared-registration failure class; the sampler records enough evidence to distinguish any remaining disk, inode, memory, or cgroup limit.After the branch merged latest main, one new bundled skill moved the widest repository review-context match from exactly 128 files to 129. The focused case failed identically on the exact base and PR head, so rerunning could not recover. Removing the unbounded sibling expansion fixes the growth coupling instead of raising the contract limit.
Later failures exposed two more test-only load dependencies. The shell-parser lazy tests performed per-case module transformation and real WASM resource resolution before reaching their mocked runtime; once a case timed out, its uncancelled initialization could mutate the next case. The workflow watchdog test also required an elapsed wall-clock value to exceed 40 ms, which is not a valid invariant on a loaded runner. Both now use deterministic boundaries rather than a longer timeout or retry.
The remaining unrelated timeouts appeared simultaneously in simple mocked tests on multiple Test jobs. Vitest 3 defaults to the forks pool, so the existing
poolOptions.threadslimit was a no-op and each suite could createavailableParallelism - 1child processes on a host shared by many runner registrations. The pool-agnosticmaxWorkerslimit now applies to the actual forks pool while retaining the intended 16-worker ceiling on a 64-core ECS host.Reviewer Test Plan
How to verify
Confirm the idle-watchdog source test fails if the finite-positive fallback assignment regresses and that the script suite has no retry. Confirm the large-fixture suite remains green while cleaning each fixture before the next test. For the CI wrapper, confirm Linux test subprocesses inherit a real short
/var/tmppath, the sampler observes that filesystem, and the wrapper returns the original test exit code on both success and failure. Confirm a skill change still selects thecore-skillsdomain without preloading every sibling skill and that the widest combined review context stays below the wire bound. Confirm the mocked shell-parser cases never read real WASM files and the abort watchdog test is independent of host wall-clock scheduling.Evidence (Before & After)
The focused idle-watchdog regression passes 10/10 independent processes without starting
run-agent; the effective script config reports retry 0. With a real short temp directory, the three socket-sensitive CLI test files pass 78/78 and the resolved-path-sensitive stale-bundle suite passes 28/28. The workflow regression suite pins both short-path routing and the failure exit-code chain across all three OS jobs. For the manifest regression, exact base and head both failed at 129/128 related files; after the change, the all-rules case resolves 86 files and the committed-policy suite passes 5/5. The UI race fixes pass 60/60 daemon rounds and 40/40 image-ingestion samples. The final shell-parser cases pass 40/40 across ten independent processes and the fake-clock watchdog case passes 10/10, with zero timeout, unhandled-rejection, retry, or flaky markers. For worker limiting, ECS-style config resolves to forks plus 3 workers on a 12-core host while hosted-style config remains unchanged; two concurrent Core processes passed 149/149 tests each and a CLI process passed 26/26, with the observed fork count capped at 3. Prettier, focused ESLint, and the affected full test files pass.Tested on
Environment (optional)
N/A.
Risk & Scope
core-skillsdomain still attaches. The remaining real packaged-WASM integration case is intentionally slower because it continues to verify the actual bundle boundary. An otherwise idle ECS runner may take longer to finish the CLI/Core suites because it now leaves CPU for neighboring jobs.Linked Issues
Observed failures: run
31556596385and today's self-hosted Test ENOSPC incidents.中文说明
本 PR 降低 required Test gate 对共享 runner 负载和临时目录压力的敏感度，不依赖失败重试。主要改动包括让 idle-watchdog、shell parser、workflow watchdog 和 UI race 测试等待可观察状态或使用可控时钟，及时清理大型 fixture，将 Linux 临时目录迁到短路径的
/var/tmp，并保留原始失败退出码。另外，分支合并最新 main 后，新增的 bundled skill 让 repository review context 从上限 128 个文件确定性增长到 129 个；exact base 和 PR head 均可复现，重跑无效。本 PR 删除了会随技能数量增长的全量 sibling expansion，同时保留
core-skills域标记。修复后最宽组合为 86 个文件，committed-policy 5/5、provider 34/34 通过。脚本测试不再配置 retry；剩余真实打包 WASM 集成 case 仍保留，用于覆盖实际 bundle 边界。review context 不再预加载全部 sibling skill 文件，但改动文件仍在 diff 中，
core-skills域也仍会附加。其余范围限定为测试和 CI 基础设施，不修改 host tmpfs 或 cgroup 配置。另外，Vitest 实际使用默认
forkspool，原来的poolOptions.threads并未限制进程数。现在只在ecs-qwen-*共享 runner 上通过原生maxWorkers限制到可用 CPU 的 25%，hosted 与本地并发保持不变。