fix(test): isolate ACP integration agents via QWEN_HOME to end parallel-settings race#5724
Conversation
…el-settings race
The ACP test `supports session/set_config_option for mode and model` flakes in
CI at acp-integration.test.ts:516 (`expect(openaiModel).toBeDefined()`).
Root cause: `globalSetup` does not sandbox HOME, so every integration test
shares the real `$HOME/.qwen`, and `vitest.config.ts` runs test files with
`fileParallelism: true` (up to 4 at once). The ACP `authenticate` / `setModel`
handlers persist `security.auth.selectedType` (and `model.name`) to User scope.
A concurrent test (e.g. system-control's `setModel('qwen3-...')`) can clobber
the persisted auth type in the window between this agent's
`authenticate({ methodId: 'openai' })` and its `session/new`; the new session
then resolves a non-openai auth, the openai runtime model is never captured, and
it drops out of `availableModels`.
Spawn each ACP agent with a per-agent `QWEN_HOME` so `getGlobalQwenDir()` points
at an isolated dir and the authenticate -> session/new round-trip reads back
exactly what this agent wrote. Test-only; no runtime code changes.
|
Thanks for the PR, @wenshao! Template looks good ✓ On direction: this is a focused test-infrastructure fix for the flaky ACP On approach: minimal and correct — one file, 20 lines, uses the same Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个聚焦的测试基础设施修复,解决一直在阻塞发布流程的 ACP 方案:最小化且正确——一个文件,20 行,采用 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: Given the problem (parallel test files sharing Comparison with PR: The PR matches this independent proposal exactly. The implementation is clean:
No correctness issues. No AGENTS.md violations. The 17-line comment block is long but justified — future maintainers debugging the same race will need exactly this context. Reuse check: Uses the existing TestingThis is a test-only change with no user-visible behavior, so tmux CLI screenshots aren't applicable. The validation is running the affected test. Before (main, without PR): After (PR applied): Interpretation: The test fails in both cases in single-file runs. This is expected and does NOT invalidate the PR. The race the PR fixes only manifests under The CI job that would validate this ( Agent stderr confirms QWEN_HOME isolation is working: The agent correctly picks up the isolated QWEN_HOME. The single-file failure appears to be a separate issue (the 中文说明代码审查独立提案: 给定问题(并行测试文件共享 与 PR 对比: PR 完全匹配这个独立提案。实现干净:
无正确性问题。无 AGENTS.md 违规。17 行注释块较长但合理——未来维护者调试同一竞态时正需要这些上下文。 测试这是仅改测试的变更,无用户可见行为,tmux CLI 截图不适用。验证方式是跑受影响的测试。 单文件运行下,PR 前后测试均失败。这符合预期,并不否定 PR。PR 修复的竞态仅在 CI 中验证此修复的 job( Agent stderr 确认 QWEN_HOME 隔离生效——agent 正确使用了隔离目录。 — Qwen Code · qwen3.7-max |
ReflectionThe PR's approach matches my independent proposal exactly — give each spawned agent its own The race-condition analysis is solid and well-evidenced: shared The code is correct: unique per-test directory, idempotent The only caveat is that I couldn't empirically validate the fix — single-file test runs don't trigger the parallel race, and the CI job that would ( Approving. ✅ 中文说明总结PR 方案与我的独立提案完全一致——给每个 spawn 的 agent 分配测试临时目录下的独立 竞态分析扎实且有据:共享 代码正确:每个测试唯一目录、幂等 唯一的保留是我无法经验性验证修复——单文件运行不触发并行竞态,CI 验证 job 仅在 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
… provider model (#5728) Follow-up to #5724. The `supports session/set_config_option for mode and model` test asserted that an openai model appears in `availableModels`, which depended on the env-driven OPENAI_MODEL being captured as a runtime-model snapshot and enumerated at session/new. That capture is environment-sensitive: in CI the openai model was absent from `availableModels`, failing `expect(openaiModel).toBeDefined()` (acp-integration.test.ts:535). The earlier QWEN_HOME isolation (#5724) did not address this. Inject an openai provider model via `modelProviders` in the test's settings so the model is a registry entry that is always enumerated and switchable without inference. The test now targets that specific model, making it deterministic regardless of how the ambient openai credentials resolve. Verified locally to pass both with and without OPENAI_MODEL set.
What this PR does
Spawns each ACP integration-test agent (
setupAcpTestinacp-integration.test.ts) with a per-agentQWEN_HOMEso its global (User-scope) qwen config dir is isolated from every other test. Test-only change; no runtime/production code is touched.Why it's needed
The ACP test
supports session/set_config_option for mode and modelflakes in CI atacp-integration.test.ts:516(expect(openaiModel).toBeDefined()), which fails the Integration Tests (No Sandbox) job and gates Publish Release. My previous PR #5721 restored the originalavailableModels.find(m => m.modelId.includes('openai'))form believing the assertion shape was the problem — but the very run triggered by that merge (run 27989509217) failed again at line 516. The assertion shape was never the real cause.Real root cause:
integration-tests/globalSetup.tsdoes not sandboxHOME, so every integration test shares the real$HOME/.qwen, andintegration-tests/vitest.config.tssetsfileParallelism: true(up to 4 test files at once). The ACPauthenticateandsetModelhandlers persistsecurity.auth.selectedType(andmodel.name) to User scope ($HOME/.qwen/settings.json). A concurrent test file — e.g.sdk-typescript/system-control.test.tscallingsetModel('qwen3-vl-plus')/setModel('qwen3-turbo')— clobbers the persisted auth type in the window between this agent'sauthenticate({ methodId: 'openai' })and itssession/new. The new per-sessionConfigreads the clobbered settings from disk, resolves a non-openai auth, never captures the openai runtime-model snapshot, and the openai model drops out ofavailableModels. BecauseformatAcpModelIdalways appends(openai),find(...includes('openai'))then returnsundefinedand the assertion fails. (#5676'savailableModels.some(m => m.modelId === currentValue)form failed at the same line for the same underlying reason — the current model is also absent from the list.)Setting a per-agent
QWEN_HOMEredirectsStorage.getGlobalQwenDir()(which honorsQWEN_HOMEfirst) to an isolated directory, so theauthenticate->session/newround-trip reads back exactly what this agent wrote, immune to any concurrent writer. This is the same isolation mechanism already used byqwen-config-dir.test.ts.Reviewer Test Plan
How to verify
npm run bundle.set_config_optionperforms no inference):Expected:
supports session/set_config_option for mode and modelandreturns error for invalid configId in set_config_optionboth pass.Mechanism evidence (driving the bundled ACP agent directly):
QWEN_HOMEand a deliberately hostile shared$HOME/.qwen/settings.json(selectedType: 'qwen-oauth',model.name: 'qwen3-turbo'),session/newstill returnsavailableModelscontaining$runtime|openai|<model>(openai)— the openai model survives → test passes.$HOME/.qwen/settings.jsontoselectedType: 'qwen-oauth'in the window betweenauthenticateandsession/newbreaks the flow (the session resolves qwen-oauth and either drops the openai model or rejects with auth-required) — reproducing the CI symptom family.returns internal error details when model auth is requiredtest (which skipsauthenticateand expects aqwen-oauthmodel in the list) still passes under isolation —coder-model(qwen-oauth)remains present.Evidence (Before & After)
N/A (test-only change, not user-visible). Before: under parallel CI load the shared
$HOME/.qwenauth type is clobbered betweenauthenticateandsession/new→ no openai model inavailableModels→acp-integration.test.ts:516fails → No-Sandbox job exits 1 → Publish Release skipped. After: each ACP agent owns an isolatedQWEN_HOME, so its persisted auth type cannot be clobbered by a concurrent test → the openai runtime model is reliably enumerated.Tested on
macOS: built the bundle and ran
supports session/set_config_option for mode and model,returns error for invalid configId in set_config_option, andreturns internal error details when model auth is requiredlocally — all pass. Windows/Linux not run locally; the authoritative check is the CI No-Sandbox job.Environment (optional)
Local:
npm run bundle+vitest run --root ./integration-testswith fakeOPENAI_*env,QWEN_SANDBOX=false.Risk & Scope
QWEN_HOMEto the test-spawned agent's env. Isolation matches the existingqwen-config-dir.test.tspattern.acp-cron.test.ts) share the same latent shared-$HOMEfragility and could adopt the same isolation in a follow-up.Linked Issues
Refs failing run https://github.com/QwenLM/qwen-code/actions/runs/27989509217/job/82838650755. Follow-up to #5721 (which addressed the assertion shape but not the underlying shared-settings race).
中文说明
这个 PR 做了什么
让每个 ACP 集成测试 agent(
acp-integration.test.ts里的setupAcpTest)在 spawn 时带上各自独立的QWEN_HOME,从而把它的全局(User 级)qwen 配置目录与其他测试隔离开。仅改测试代码,不涉及任何运行时/生产代码。为什么需要
ACP 测试
supports session/set_config_option for mode and model在 CI 的acp-integration.test.ts:516(expect(openaiModel).toBeDefined())随机失败,会让 Integration Tests (No Sandbox) job 失败,而它 gating Publish Release。我上一个 PR #5721 把断言恢复成原始的availableModels.find(m => m.modelId.includes('openai'))写法,以为问题出在断言形态——但那次合并触发的 run(27989509217)依然在 516 行失败。断言形态从来不是真正的根因。真正的根因:
integration-tests/globalSetup.ts没有隔离HOME,所有集成测试共享真实的$HOME/.qwen;而integration-tests/vitest.config.ts设了fileParallelism: true(最多 4 个测试文件并行)。ACP 的authenticate和setModelhandler 会把security.auth.selectedType(以及model.name)持久化到 User 级 的$HOME/.qwen/settings.json。某个并行的测试文件——例如sdk-typescript/system-control.test.ts调用setModel('qwen3-vl-plus')/setModel('qwen3-turbo')——会在本测试authenticate({ methodId: 'openai' })与session/new之间的窗口里把持久化的 auth type 改掉。新建会话的Config从磁盘读到被改写的设置,解析成非 openai 鉴权,于是 openai 运行时模型快照根本没被捕获,openai 模型也就从availableModels里消失。由于formatAcpModelId总是拼上(openai),find(...includes('openai'))返回undefined,断言失败。(#5676 的availableModels.some(m => m.modelId === currentValue)写法在同一行失败,根因相同——当前模型同样不在列表里。)给每个 agent 设置独立的
QWEN_HOME,会让Storage.getGlobalQwenDir()(它优先读取QWEN_HOME)指向一个隔离目录,于是authenticate->session/new的往返读到的就是这个 agent 自己写入的内容,不受任何并发写入者影响。这正是qwen-config-dir.test.ts已经在用的隔离手段。评审验证
如何验证
npm run bundle。set_config_option不做任何推理)对着 bundle 跑之前易挂的用例:预期:
supports session/set_config_option for mode and model与returns error for invalid configId in set_config_option均通过。机制佐证(直接驱动打包后的 ACP agent):
QWEN_HOME,并且故意把共享的$HOME/.qwen/settings.json写成敌对值(selectedType: 'qwen-oauth'、model.name: 'qwen3-turbo')时,session/new返回的availableModels里仍然包含$runtime|openai|<model>(openai)——openai 模型存活 → 用例通过。authenticate与session/new之间把共享的$HOME/.qwen/settings.json覆写成selectedType: 'qwen-oauth',整个流程就坏掉(会话解析成 qwen-oauth,要么丢掉 openai 模型,要么直接以 auth-required 报错)——复现了 CI 故障所属的同一类。returns internal error details when model auth is required用例(不调用authenticate,期望列表里有 qwen-oauth 模型)在隔离下依然通过——coder-model(qwen-oauth)仍在列表中。证据(Before & After)
N/A(仅改测试,无用户可见变化)。Before:并行 CI 负载下,共享的
$HOME/.qwenauth type 在authenticate与session/new之间被改掉 →availableModels里没有 openai 模型 →acp-integration.test.ts:516失败 → No-Sandbox job 退出码 1 → Publish Release 被跳过。After:每个 ACP agent 拥有独立QWEN_HOME,持久化的 auth type 不会被并发测试改写 → openai 运行时模型稳定出现在列表里。测试平台
macOS:构建 bundle 后本地跑了
supports session/set_config_option for mode and model、returns error for invalid configId in set_config_option、returns internal error details when model auth is required,全部通过。Windows/Linux 未在本地运行;以 CI 的 No-Sandbox job 为权威校验。运行环境(可选)
本地:
npm run bundle+vitest run --root ./integration-tests,使用假的OPENAI_*环境变量、QWEN_SANDBOX=false。风险与范围
QWEN_HOME,隔离方式与现有的qwen-config-dir.test.ts一致。acp-cron.test.ts)有同样潜在的共享$HOME脆弱性,可在后续 PR 采用相同隔离。关联
关联失败 run https://github.com/QwenLM/qwen-code/actions/runs/27989509217/job/82838650755。是 #5721 的后续(#5721 只动了断言形态,没解决底层的共享设置竞态)。