fix(tests): await rig setup in Qoder plugin install integration test - #8793
Merged
Conversation
Collaborator
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The linked issue #8766 has seven recurrences of the exact same failure ( Direction: aligned — a test-race fix that unblocks the nightly release pipeline, with no product behavior change. Size: not applicable — one line in one integration test file, no core paths touched. Approach: the scope is exactly right. Risk: no elevated risk signals (no high-risk paths matched). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的真实问题，不是理论性的。关联的 #8766 自 8 月 8 日以来在 方向：对齐——这是一个测试竞态修复，解除对 nightly release 流水线的阻塞，不涉及任何产品行为变化。 规模：不适用——单个集成测试文件中的一行改动，未触及核心路径。 方案：范围恰好。 风险：无升级风险信号（未命中高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewBefore reading the diff, my independent take from the description: the failure is The mechanism checks out against No correctness, security, or convention issues. This was the only un-awaited Testing evidence (this PR's CI — unattended run, no local execution)The
Author-reported: Sandboxed verification could settle the remaining gap: 中文说明代码审查读 diff 之前，我基于描述的独立判断是：Qoder 插件用例报 机理与 无正确性、安全性或规范问题。这是整个集成测试套件中唯一漏掉 await 的 测试证据（本 PR 的 CI——无人值守运行，不在本地执行）该 commit 上的 （CI 表格见上方标记区域） 作者自述：本地 macOS 上 沙箱验证可以补上剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — a one-line, precisely targeted fix for a well-evidenced CI race; nothing extra in the diff, and the PR's CI gate is green. Stepping back: the problem is real and costly (seven recurrences on The one honest caveat: no PR-side CI job executes the changed test (the integration suite runs in the merge queue and post-merge), so the green signal for this specific test arrives after merge — that's a property of the CI setup, not of this PR. The await is a static ordering guarantee, which is what makes me comfortable approving without that run. Approving, pinned to the reviewed commit. 中文说明置信度：5/5 —— 针对证据充分的 CI 竞态的一行精准修复，diff 中没有任何多余内容，且 PR 的 CI 门禁为绿色。 整体来看：问题真实且代价不小（ 唯一如实说明的保留：PR 侧 CI 没有任何 job 会执行被改动的测试（集成套件在 merge queue 和合并后运行），所以这个具体测试的绿色信号要到合并后才会出现——这是 CI 配置的性质，不是这个 PR 的问题。await 本身是静态的顺序保证，这也是我在没有该运行的情况下仍放心批准的原因。 予以批准，锚定到本次审查的 commit。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Local verification report (maintainer)Built the PR head ( What I ran
Same machine, same bundle, same command (
Failure signature matches CI verbatimThe local repro of B fails with exactly the error from the
Filesystem forensics — direct evidence of the raceRan each variant once with
Root cause provenance (why only this test flakes)
VerdictMinimal, correct, and matches the sibling test's existing pattern. Local evidence: deterministic 0/10 → 10/10 flip on the single changed line, failure signature identical to the CI/nightly failures in #8766 / #8771. LGTM. 中文版本（Chinese version）本地验证报告（维护者）在隔离 worktree 中从源码构建了 PR head（ 实验矩阵
同一台机器、同一份 bundle、同一条命令（ 失败信息与 CI 逐字一致B 变体的本地失败报错与 #8766 最新一次复发（run 31298084187 的 文件系统取证——竞态的直接证据对两个变体各以 根因来源（为什么只有这个测试会抖）
结论改动最小、修复正确、与旁边兄弟测试的既有写法一致。本地证据：单行 A/B 对照 0/10 → 10/10 的确定性翻转，失败签名与 #8766 / #8771 的 CI 及 nightly 失败完全一致。LGTM。 Evidence screenshots hosted on branch |
doudouOUC
approved these changes
Aug 9, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Approving — reviewed at 927d6e04f47f550a17e211cd8000266827a2c6b4. One line, correct, and the mechanism is verifiable rather than plausible. I reproduced it instead of taking it on faith.
The race is real, and deterministic on my machine
TestRig.setup assigns this.testDir synchronously, then await rm(this.testDir, { recursive: true, force: true }), then recreates the directory and writes .qwen/settings.json. That ordering is why the un-awaited call looked harmless: rig.testDir! on the next line is already populated, so the test body proceeds happily while the recursive delete is still in flight.
I built a standalone probe with the same shape — sync dir assignment, await rm(recursive), mkdirSync, writeFileSync, called without await, with the test body writing .qoder-plugin/plugin.json right after — and ran it three times:
un-awaited → manifest survives: false | settings written: true
awaited → manifest survives: true | settings written: true
un-awaited → manifest survives: false | settings written: true
awaited → manifest survives: true | settings written: true
un-awaited → manifest survives: false | settings written: true
awaited → manifest survives: true | settings written: true
Note the second column: setup's own later writes do land, so the failure surfaces exactly as reported — the fixtures are gone while the environment looks fine, hence Configuration file not found for the manifest and nothing more informative. 3/3 un-awaited runs wiped the fixtures; 3/3 awaited runs kept them.
The sweep is complete
I checked this independently of the claim: after this diff, every .setup( call site under integration-tests/ is awaited. The only remaining textual match is a comment in settings-migration.test.ts (This overwrites the settings file created by rig.setup().). So there is no second instance of this bug waiting in another file, and no follow-up sweep is owed.
Also worth stating: awaiting has a second effect beyond the fixture race — an rm/write rejection inside setup now propagates into the test instead of becoming an unhandled rejection attributed to nothing.
Two notes, neither blocking
The changed test does not run in this PR's CI. The integration suite is a merge-queue/post-merge lane, so the green Test (ubuntu-latest, Node 22.x) here says nothing about the case being fixed. I am comfortable anyway: an await is a static ordering guarantee, and the probe above is the behavioural evidence a single CI run could not have given either (the race is load-dependent — a green run proves nothing about the next one).
The triage check on this commit failed, and the bot's stage-3 comment says "Approving, pinned to the reviewed commit" while the PR has no review from it — its approval never landed. Nothing to do with your diff; flagging it so the failed check isn't mistaken for a signal about this change, and so someone re-runs it if that gate is required for merge.
Optional follow-up, explicitly out of scope here: nothing static could have caught this class — the repo runs tseslint.configs.recommended, not the type-checked variant, so @typescript-eslint/no-floating-promises is not enabled anywhere, and the integration-tests ESLint block only sets globals. Turning it on for that directory would need type-aware linting wired up for it, which is real cost and absolutely not this PR's job — but the class just cost a nightly release, so it may be worth its own issue.
中文说明
批准 —— 审查提交 927d6e04f47f550a17e211cd8000266827a2c6b4。一行改动，正确，且机理可验证而非仅仅"看起来合理"。我复现了它，而不是照抄结论。
竞态是真实的，在我机器上可稳定复现
TestRig.setup 先同步赋值 this.testDir，然后 await rm(this.testDir, { recursive: true, force: true })，再重建目录并写入 .qwen/settings.json。这个顺序正是"漏掉 await 看起来没事"的原因：下一行的 rig.testDir! 已经有值，测试体于是照常往下走，而递归删除仍在飞行中。
我写了一个同形态的独立探针——同步赋值目录、await rm(recursive)、mkdirSync、writeFileSync，调用处不加 await，紧接着由测试体写入 .qoder-plugin/plugin.json——跑了三次：
不加 await → manifest 存活: false | settings 已写入: true
加 await → manifest 存活: true | settings 已写入: true
（三轮结果一致）
注意第二列：setup 自己后续的写入是落盘的，所以失败表现与报告完全一致——夹具消失了，但环境看起来正常，于是只报 Configuration file not found，没有更多线索。不加 await 三轮全部被清空；加 await 三轮全部存活。
清扫是完整的
这一点我独立核对过：本 diff 之后，integration-tests/ 下每一个 .setup( 调用点都已 await。唯一残留的文本匹配是 settings-migration.test.ts 里的一句注释。因此不存在潜伏在其他文件里的第二处同类缺陷，也不欠后续清扫。
另外值得一提：加上 await 还有第二个效果——setup 内部 rm/写入失败现在会传播到测试里，而不是变成无归属的 unhandled rejection。
两点说明，均不阻塞
被改动的测试在本 PR 的 CI 中不会运行。 集成套件属于 merge-queue / 合并后的通道，因此这里绿色的 Test (ubuntu-latest, Node 22.x) 对被修复的用例毫无说明力。但我仍然放心：await 是静态的顺序保证，而上面的探针给出的行为证据，恰恰是单次 CI 运行也给不出的（竞态依赖负载——一次绿色不能证明下一次）。
本 commit 上的 triage 检查失败了，而机器人 stage-3 评论写着"予以批准，锚定到本次审查的 commit"，但 PR 上并没有它的 review——它的批准从未落地。这与你的 diff 无关；指出来是为了避免把这个失败的检查误读成对本改动的信号，并且如果该门禁是合并必需项，需要有人重跑。
可选后续，明确不属于本 PR： 没有任何静态检查能捕获这一类问题——仓库用的是 tseslint.configs.recommended 而非 type-checked 变体，因此 @typescript-eslint/no-floating-promises 在任何地方都没启用，而 integration-tests 的 ESLint 配置块只设置了 globals。为该目录启用它需要接入类型感知 lint，成本真实存在，也绝不是本 PR 该做的事——但这一类问题刚刚造成了一次 nightly release 失败，也许值得单独开 issue。
— reviewed by doudouOUC
wenshao
reviewed
Aug 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| test('installs a local Qoder plugin', async () => { | ||
| const rig = new TestRig(); | ||
| rig.setup('qoder plugin install test'); | ||
| await rig.setup('qoder plugin install test'); |
Collaborator
There was a problem hiding this comment.
[Suggestion] No per-PR test gate exercises this file — Failure scenario: the changed file sits outside every npm workspace, so no npm-workspace test command collects it; the per-PR build/test gate ran nothing (verified: empty build/test scope, test-efficacy classified the file unreachable). Only the merge_group-gated integration_cli job (which per its own ci.yml comment "simply never triggers" until merge queue is enabled), the release pipeline, and e2e runs (push-to-main + nightly cron) exercise it. Reverting this await would still pass locally on a fast machine (the race is load-dependent), so a regression in this test can land on main with all PR checks green. Suggested fix: confirm the integration_cli merge-queue gate is actually enforced so this file gets per-PR CI coverage.
中文说明
建议 — 该文件的修改没有任何 per-PR 测试门禁覆盖。失败场景：被改文件不在任何 npm workspace 内，没有 npm-workspace 测试命令会收集它；per-PR 的 build/test 门禁实际什么都没跑（已验证 build/test 范围为空的，test-efficacy 将该文件判为 unreachable）。只有 merge_group 门控的 integration_cli 任务（按 ci.yml 自身注释，在启用 merge queue 前"永远不会触发"）、发布流水线和 e2e（push 到 main + nightly cron）会执行它。回退这个 await 在本地快机器上仍会通过（竞态依赖负载），因此该测试的回归可能带着全绿的 PR 检查进入 main。建议：确认 integration_cli 的 merge queue 门控真正生效，让该文件获得 per-PR CI 覆盖。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| test('installs a local Qoder plugin', async () => { | ||
| const rig = new TestRig(); | ||
| rig.setup('qoder plugin install test'); | ||
| await rig.setup('qoder plugin install test'); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The fix leaves no in-test tripwire, so a future reintroduction of the bare call fails with a misleading generic error instead of at the setup site — Failure scenario: TestRig.setup() assigns this.testDir synchronously and does all real work (rm of the stale dir, re-mkdirSync, writeFileSync of .qwen/settings.json) in its async continuation. If a future edit or merge-conflict resolution drops the await again, the test body's synchronous mkdirSync/writeFileSync of the manifest files completes first, then the floating continuation's rm(testDir, { recursive: true }) deletes exactly those just-written files and recreates the dir empty before extensions install runs — the install then finds no manifest and the test fails with a generic exit-code error that gives no hint the cause was the missing await. A probe of the mechanism lost the manifest 30/30 runs in the buggy arm vs 0/30 in the fixed arm. Suggested fix: add a one-line tripwire right after setup that asserts the last thing setup() writes — false at that instant iff the await is missing:
Suggested change
| await rig.setup('qoder plugin install test'); | |
| await rig.setup('qoder plugin install test'); | |
| expect(existsSync(join(rig.testDir!, '.qwen/settings.json'))).toBe(true); |
中文说明
建议 — 这个修复没有在测试内部留下"绊线"，将来若 await 被再次去掉，测试会在 install 步骤以一个误导性的通用错误失败，而不是在 setup 处失败。失败场景：TestRig.setup() 同步先赋值 this.testDir，所有实际工作（删除旧目录、重新 mkdirSync、写入 .qwen/settings.json）都在其异步续体中完成。如果将来的编辑或合并冲突解决再次丢掉 await，测试体同步的 mkdirSync/writeFileSync 先写完 manifest 文件，然后浮动续体中的 rm(testDir, { recursive: true }) 恰好删除这些刚写好的文件并重建空目录，接着 extensions install 找不到 manifest，测试以通用退出码错误失败，无法提示缺失 await 才是原因。机制探针显示 bug 分支 30/30 次丢失 manifest，修复分支 0/30。建议：在 setup 之后加一行断言 setup 写入的最后一个文件——当且仅当缺少 await 时该断言在那一刻为假。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
Collaborator
|
Correction to the review body above — the auto-composed review summary was emitted from stale temp state after cleanup and wrongly claimed the diff was not reviewed. It was fully reviewed (coverage 1/1 chunk, 20 agents, reverse audit converged after 4 rounds). Correct summary: Reviewed — no blockers. Suggestions are inline. 中文说明已审查——无阻断问题。 建议见行内评论。 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8) |
wenshao
approved these changes
Aug 9, 2026
Collaborator
|
Released in v0.21.9. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Awaits the test-rig setup call in the Qoder plugin install integration test, matching the sibling extension install test which already awaits it. The setup routine clears and recreates the test directory before each case; without the await, that recursive delete runs concurrently with the test body writing the plugin manifest fixtures, so on a loaded runner the delete can land after the fixtures exist and wipe them out.
Why it's needed
The nightly release and main CI keep failing on this test with
Configuration file not foundfor the extension manifest. The failure is a load-dependent race inside the test itself — the install code path works correctly whenever the fixtures survive — so it flaps across runs and blocks releases without any product defect. This was the root cause behind the #8766 CI failure and the failed v0.21.8 nightly release tracked in #8771.Reviewer Test Plan
How to verify
Build the bundle and run the affected integration test without sandbox:
npm run build && npm run bundle, thencd integration-tests && QWEN_SANDBOX=false npx vitest run cli/extensions-install.test.ts. Both cases should pass deterministically. Verified locally: 2 passed (2), the Qoder plugin case in ~2.2s.Evidence (Before & After)
N/A — test-only change, no user-visible behavior.
Tested on
Environment (optional)
Local integration test run against the bundled CLI with
QWEN_SANDBOX=false.Risk & Scope
Linked Issues
Fixes #8766
Related: #8771 (same root cause behind the failed nightly release)
中文说明
这个 PR 做了什么
给 Qoder 插件安装集成测试中的测试夹具初始化调用补上缺失的
await，与旁边已经正确 await 的扩展安装测试保持一致。夹具初始化会在每个用例开始前清空并重建测试目录；不 await 的话，这个递归删除会与测试体写入插件 manifest 夹具的操作并发执行，在负载较高的 runner 上，删除可能落在夹具写入之后，把刚写好的文件删掉。为什么需要
nightly release 和主 CI 反复因为这个测试报
Configuration file not found（找不到扩展 manifest）。这是测试自身的负载敏感竞态——只要夹具存活，安装路径本身完全正常——所以它在不同 run 之间时好时坏，在没有产品缺陷的情况下阻塞发布。#8766 的 CI 失败和 #8771 跟踪的 v0.21.8 nightly release 失败，根因都是它。评审验证计划
如何验证
构建 bundle 后在无沙箱模式下运行受影响的集成测试：
npm run build && npm run bundle，然后cd integration-tests && QWEN_SANDBOX=false npx vitest run cli/extensions-install.test.ts。两个用例应稳定通过。本地已验证：2 passed (2)，其中 Qoder 插件用例约 2.2 秒。前后对比证据
N/A——纯测试改动，无用户可见行为变化。
测试环境
macOS 已验证；Windows / Linux 依赖 CI。
运行环境（可选）
本地以
QWEN_SANDBOX=false对打包后的 CLI 运行集成测试。风险与范围
关联 Issue
Fixes #8766
相关：#8771（nightly release 失败的同一根因）