fix(tests): avoid blocking the Vitest worker during directory E2E - #8685
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E Test ReportEnglishScenarioRequired no-AK integration gate on Linux, focusing on the Change under testRemoved the test-only full-system Results
Before / After
中文场景Linux required no-AK integration gate,重点验证此前耗时约 83 秒、并在 130 个断言全部通过后触发 Vitest 3.2.4 验证改动移除启动 CLI 前仅用于测试的全系统 结果
Before / After
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — the mechanism checks out in code. Direction: aligned — this is the reliability of the required no-AK integration gate itself, and the change removes latency without touching any product code. No CHANGELOG signal applies to a test-infra fix. Size: not applicable — test-only change, 0 additions / 6 deletions across 2 files under Approach: the scope is exactly right. Fixture creation stays synchronous ( Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的问题,不是理论性加固——机制在代码中可以确认。 方向:对齐——这修的是 required no-AK integration gate 自身的可靠性,且只移除延迟,不涉及任何产品代码。测试基建修复不涉及 CHANGELOG 信号。 规模:不适用——纯测试改动(2 个文件,0 增 / 6 删,均在 方案:范围恰好。夹具创建保持同步( 风险:无升级风险信号——未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers. My independent take before reading the diff: the fixture writes are already synchronous and the test polls for both paths before launching the CLI, so a system-wide flush buys durability that no scenario here needs — the minimal fix is to delete the call and the now-unused helper. That is exactly what this diff does, and nothing more. Two details check out: (The author's local numbers — 5/5 focused runs at ~1.3s and a CI-shaped gate run at 1.625s without the RPC timeout — are their own report, attributed as such; the evidence below is the PR's CI, per gate policy.) Test evidence — this PR's own CIThe no-AK integration gate runs inside the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Not verified: local reruns on Windows/Linux — the author validated on macOS only. That gap does not weaken the claim here, because the claim is about the CI gate and this PR's CI exercises it on Linux. Nothing user-visible changes, so no real-scenario TUI run applies (N/A). 中文说明代码审查无阻塞问题。我在看 diff 之前的独立判断:夹具写入本身已经是同步的,测试在启动 CLI 前会对两个路径做轮询,因此全系统刷新换来的持久化保证在这里没有任何场景需要——最小修复就是删掉调用和随之不再使用的 helper。diff 与这个判断完全一致,且没有任何多余改动。两个细节成立: (作者的本地数据——目标测试 5/5 通过、每次约 1.3 秒,CI 形态 gate 运行 1.625 秒且未再现 RPC 超时——是其自述,仅作为引述;按 gate 规则,下方证据来自该 PR 自身的 CI。) 测试证据——该 PR 自身的 CIno-AK integration gate 运行于 (CI 状态表见英文部分,此处不重复。) 未验证:Windows/Linux 本地复跑——作者仅在 macOS 上验证。这个缺口不影响本 PR 的主张,因为主张针对的是 CI gate,而本 PR 的 CI 正在 Linux 上运行它。无用户可见变化,真实场景 TUI 验证不适用(N/A)。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — minimal, mechanic-backed fix for a real gate flake; the only open item is CI landing green, which the deferred approval waits for. Stepping back: this removes a synchronous full-system disk flush that can block the Vitest worker's event loop past birpc's 60-second RPC timeout on busy self-hosted runners, failing a required gate even when every assertion passes. The diff is the minimal change — the one call removed from the one test that made it, the helper deleted with its last caller, and nothing else. The test's behavior and assertions are unchanged; the readiness boundary stays the real-condition poll, which is the correct one. My independent proposal before reading the diff was exactly this, and I found nothing simpler and nothing it missed. The author is a maintainer who sees this gate fail in CI, and even setting the reported run aside, the change strictly removes a blocking call whose flush guarantee no scenario here relies on — there is no downside case. Once CI confirms the gate is green on this commit, nothing should hold it back. Approval deferred until CI lands green on 中文说明Confidence: 5/5 —— 针对真实 gate flake 的最小化、机制明确的修复;唯一的待办是等 CI 变绿,延迟批准会等待该结果。 退一步看:这个改动移除了同步的全系统磁盘刷新——它在繁忙的 self-hosted runner 上可能阻塞 Vitest worker 的事件循环,超过 birpc 的 60 秒 RPC 超时,导致即使所有断言都通过,required gate 仍然失败。diff 是最小改动——唯一使用该方法的测试删掉了调用,helper 随最后一个调用方一并删除,没有任何其他改动。测试的行为与断言均未变化,就绪边界仍是真实条件轮询,这才是正确的边界。我在读 diff 之前的独立提议与此完全一致,既没有更简的路径,也没有发现遗漏。作者是亲眼看到该 gate 在 CI 中失败的 maintainer;即使搁置其报告的运行不谈,这个改动也只是严格移除了一个阻塞调用——其刷新保证在这里没有任何场景依赖,不存在 downside。CI 确认本 commit 的 gate 为绿之后,不应再有任何阻碍。 CI 在该 commit 上变绿后完成批准。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Code reviewThe deletion itself is safe, but the justification in the description is not the one that actually holds — and one of the two things the description leans on is a no-op. Three notes, then two nits. 1. The retained poll is vacuous, so it cannot be the "readiness boundary"The description says the flush is redundant because "the test separately waits for both created paths to be visible". It does not. The reason the removal is safe is a different one, and a stronger one: Either state that reasoning and delete the poll as dead code too, or — if the poll is meant to guard what the spawned CLI sees rather than what the runner process sees — make it check that. 2. This is a revert of a deliberate flake mitigation, and
|
wenshao
left a comment
There was a problem hiding this comment.
中文说明
— gpt-5.6-sol via Qwen Code /review (v0.21.7)
|
Released in v0.21.8. |
What this PR does
Removes an unnecessary full-system disk flush from the directory-listing integration test while preserving synchronous fixture creation, the existing readiness poll, and every behavioral assertion. The unused test helper entry point is removed as well.
Why it's needed
On a busy self-hosted Linux runner, the synchronous
synccommand can block the Vitest worker event loop for more than birpc's 60-second RPC timeout. The test itself can still pass, but Vitest then reports[vitest-worker]: Timeout calling "onTaskUpdate"and fails the required no-AK gate. The existing filesystem operations already complete synchronously, and the test separately waits for both created paths to be visible, so the global flush adds latency without adding correctness.Reviewer Test Plan
How to verify
Build and bundle the CLI, then run
npx vitest run --root ./integration-tests --maxWorkers 2 ./cli/list_directory.test.tsrepeatedly. Confirm the test continues to validate the tool call and both directory entries without invoking the system-widesynccommand. Run the required no-AK integration gate in an isolated HOME with credential variables cleared and confirm the directory-listing test completes promptly without the Vitest worker RPC timeout.Local evidence: the focused test passed 5/5 runs at about 1.3 seconds per run. In a CI-shaped full-gate run it passed in 1.625 seconds and the prior RPC timeout did not recur. The full local gate was still red because two unrelated permission-control assertions fail consistently on the current local main baseline (128/130 tests passed); those failures are outside this change.
Evidence (Before & After)
Before: the affected CI run completed the directory-listing test after about 83 seconds, then failed with an unhandled
[vitest-worker]: Timeout calling "onTaskUpdate"despite all 130 assertions passing.After: five focused runs completed in about 1.3 seconds each, and the CI-shaped full-gate run completed the directory-listing test in 1.625 seconds without the worker RPC timeout.
Tested on
Environment (optional)
macOS 15, Node.js 22.22.2, Vitest 3.2.4, no sandbox. Linux behavior is represented by the original GitHub Actions failure but was not rerun locally on Linux.
Risk & Scope
Linked Issues
N/A
中文说明
What this PR does
移除目录列表 integration test 中不必要的全系统磁盘刷新,同时保留同步创建测试夹具、现有就绪条件轮询以及全部行为断言,并删除不再使用的测试 helper 入口。
Why it's needed
在繁忙的 self-hosted Linux runner 上,同步
sync命令可能阻塞 Vitest worker 事件循环超过 birpc 的 60 秒 RPC timeout。测试本身仍可通过,但 Vitest 随后会报告[vitest-worker]: Timeout calling "onTaskUpdate",导致 required no-AK gate 失败。现有文件系统操作本身已经同步完成,而且测试还会单独等待两个创建路径真实可见,因此全局刷新只增加延迟,不增加正确性保障。Reviewer Test Plan
How to verify
构建并 bundle CLI,然后重复运行
npx vitest run --root ./integration-tests --maxWorkers 2 ./cli/list_directory.test.ts。确认测试在不调用系统级sync的情况下,仍验证工具调用和两个目录条目。使用隔离 HOME 并清空凭据环境变量运行 required no-AK integration gate,确认目录列表测试快速完成,且不再出现 Vitest worker RPC timeout。本地证据:目标测试连续运行 5 次全部通过,每次约 1.3 秒。在 CI 形态的完整 gate 中,该测试耗时 1.625 秒且未再出现原 RPC timeout。完整本地 gate 仍为红色,因为当前本地主线上的两个无关 permission-control 断言稳定失败(130 个测试中 128 个通过);这些失败不在本次改动范围内。
Evidence (Before & After)
Before:受影响 CI run 中目录列表测试约 83 秒后完成;尽管 130 个断言全部通过,随后仍因未处理的
[vitest-worker]: Timeout calling "onTaskUpdate"而失败。After:目标测试连续五次均约 1.3 秒完成;CI 形态的完整 gate 中该测试耗时 1.625 秒,未出现 worker RPC timeout。
Tested on
Environment (optional)
macOS 15、Node.js 22.22.2、Vitest 3.2.4、无 sandbox。Linux 行为证据来自原始 GitHub Actions 失败,未在本地 Linux 环境重新运行。
Risk & Scope
Linked Issues
N/A