Skip to content

fix(test): isolate ACP integration agents via QWEN_HOME to end parallel-settings race#5724

Merged
yiliang114 merged 1 commit into
QwenLM:mainfrom
wenshao:fix/acp-qwen-home-isolation
Jun 23, 2026
Merged

fix(test): isolate ACP integration agents via QWEN_HOME to end parallel-settings race#5724
yiliang114 merged 1 commit into
QwenLM:mainfrom
wenshao:fix/acp-qwen-home-isolation

Conversation

@wenshao

@wenshao wenshao commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Spawns each ACP integration-test agent (setupAcpTest in acp-integration.test.ts) with a per-agent QWEN_HOME so 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 model flakes in CI at acp-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 original availableModels.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.ts does not sandbox HOME, so every integration test shares the real $HOME/.qwen, and integration-tests/vitest.config.ts sets fileParallelism: true (up to 4 test files at once). The ACP authenticate and setModel handlers persist security.auth.selectedType (and model.name) to User scope ($HOME/.qwen/settings.json). A concurrent test file — e.g. sdk-typescript/system-control.test.ts calling setModel('qwen3-vl-plus') / setModel('qwen3-turbo') — clobbers the persisted auth type in the window between this agent's authenticate({ methodId: 'openai' }) and its session/new. The new per-session Config reads the clobbered settings from disk, resolves a non-openai auth, never captures the openai runtime-model snapshot, and the openai model drops out of availableModels. Because formatAcpModelId always appends (openai), find(...includes('openai')) then returns undefined and the assertion fails. (#5676's availableModels.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_HOME redirects Storage.getGlobalQwenDir() (which honors QWEN_HOME first) to an isolated directory, so the authenticate -> session/new round-trip reads back exactly what this agent wrote, immune to any concurrent writer. This is the same isolation mechanism already used by qwen-config-dir.test.ts.

Reviewer Test Plan

How to verify

  1. Build the bundle: npm run bundle.
  2. Run the previously-flaky test against the bundle with openai env (fake creds are fine — set_config_option performs no inference):
cd integration-tests && OPENAI_API_KEY=sk-fake OPENAI_BASE_URL=https://api.openai.com/v1 OPENAI_MODEL=gpt-4o-mini-fake QWEN_SANDBOX=false \
  npx vitest run cli/acp-integration.test.ts -t "config_option"

Expected: supports session/set_config_option for mode and model and returns error for invalid configId in set_config_option both pass.

Mechanism evidence (driving the bundled ACP agent directly):

  • With a per-agent QWEN_HOME and a deliberately hostile shared $HOME/.qwen/settings.json (selectedType: 'qwen-oauth', model.name: 'qwen3-turbo'), session/new still returns availableModels containing $runtime|openai|<model>(openai) — the openai model survives → test passes.
  • Without isolation, overwriting the shared $HOME/.qwen/settings.json to selectedType: 'qwen-oauth' in the window between authenticate and session/new breaks the flow (the session resolves qwen-oauth and either drops the openai model or rejects with auth-required) — reproducing the CI symptom family.
  • Control: the returns internal error details when model auth is required test (which skips authenticate and expects a qwen-oauth model 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/.qwen auth type is clobbered between authenticate and session/new → no openai model in availableModelsacp-integration.test.ts:516 fails → No-Sandbox job exits 1 → Publish Release skipped. After: each ACP agent owns an isolated QWEN_HOME, so its persisted auth type cannot be clobbered by a concurrent test → the openai runtime model is reliably enumerated.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

macOS: built the bundle and ran supports session/set_config_option for mode and model, returns error for invalid configId in set_config_option, and returns internal error details when model auth is required locally — 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-tests with fake OPENAI_* env, QWEN_SANDBOX=false.

Risk & Scope

  • Main risk or tradeoff: None for production — the change only adds QWEN_HOME to the test-spawned agent's env. Isolation matches the existing qwen-config-dir.test.ts pattern.
  • Not validated / out of scope: Inference-driven ACP tests (smoke test, plan-mode, usage-metadata) were not re-run locally (need real provider auth); they only become more deterministic under isolation. Sibling ACP files (acp-cron.test.ts) share the same latent shared-$HOME fragility and could adopt the same isolation in a follow-up.
  • Breaking changes / migration notes: None.

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:516expect(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 的 authenticatesetModel handler 会把 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,断言失败。(#5676availableModels.some(m => m.modelId === currentValue) 写法在同一行失败,根因相同——当前模型同样不在列表里。)

给每个 agent 设置独立的 QWEN_HOME,会让 Storage.getGlobalQwenDir()(它优先读取 QWEN_HOME)指向一个隔离目录,于是 authenticate -> session/new 的往返读到的就是这个 agent 自己写入的内容,不受任何并发写入者影响。这正是 qwen-config-dir.test.ts 已经在用的隔离手段。

评审验证

如何验证

  1. 构建 bundle:npm run bundle
  2. 用 openai 环境变量(假 key 即可,set_config_option 不做任何推理)对着 bundle 跑之前易挂的用例:
cd integration-tests && OPENAI_API_KEY=sk-fake OPENAI_BASE_URL=https://api.openai.com/v1 OPENAI_MODEL=gpt-4o-mini-fake QWEN_SANDBOX=false \
  npx vitest run cli/acp-integration.test.ts -t "config_option"

预期:supports session/set_config_option for mode and modelreturns 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 模型存活 → 用例通过。
  • 不做隔离、在 authenticatesession/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/.qwen auth type 在 authenticatesession/new 之间被改掉 → availableModels 里没有 openai 模型 → acp-integration.test.ts:516 失败 → No-Sandbox job 退出码 1 → Publish Release 被跳过。After:每个 ACP agent 拥有独立 QWEN_HOME,持久化的 auth type 不会被并发测试改写 → openai 运行时模型稳定出现在列表里。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

macOS:构建 bundle 后本地跑了 supports session/set_config_option for mode and modelreturns error for invalid configId in set_config_optionreturns 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

风险与范围

  • 主要风险/取舍:对生产代码零风险——只在测试 spawn 的 agent env 里加了 QWEN_HOME,隔离方式与现有的 qwen-config-dir.test.ts 一致。
  • 未验证/范围外:依赖推理的 ACP 用例(smoke test、plan-mode、usage-metadata)未在本地重跑(需真实 provider 鉴权),它们在隔离下只会更稳定。同类 ACP 文件(acp-cron.test.ts)有同样潜在的共享 $HOME 脆弱性,可在后续 PR 采用相同隔离。
  • 破坏性变更:无。

关联

关联失败 run https://github.com/QwenLM/qwen-code/actions/runs/27989509217/job/82838650755。是 #5721 的后续(#5721 只动了断言形态,没解决底层的共享设置竞态)。

…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.
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @wenshao!

Template looks good ✓

On direction: this is a focused test-infrastructure fix for the flaky ACP set_config_option test that's been blocking the release pipeline. The root-cause analysis is convincing — the shared $HOME/.qwen/settings.json getting clobbered between authenticate and session/new by a concurrent setModel call is exactly the kind of cross-test interference that parallel file execution invites. Aligns well with project stability goals.

On approach: minimal and correct — one file, 20 lines, uses the same QWEN_HOME isolation pattern already established by qwen-config-dir.test.ts. No production code touched. The scope is exactly what the problem demands.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

方向:这是一个聚焦的测试基础设施修复,解决一直在阻塞发布流程的 ACP set_config_option 测试抖动。根因分析令人信服——并发 setModel 调用在 authenticatesession/new 之间改写了共享的 $HOME/.qwen/settings.json,正是并行文件执行会引发的跨测试干扰。与项目稳定性目标一致。

方案:最小化且正确——一个文件,20 行,采用 qwen-config-dir.test.ts 已经建立的 QWEN_HOME 隔离模式。未触及生产代码。范围恰好匹配问题所需。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: Given the problem (parallel test files sharing $HOME/.qwen/settings.json), I'd give each spawned agent its own QWEN_HOME pointing into the per-test temp directory — exactly what qwen-config-dir.test.ts already does. One-line env addition, one mkdirSync, done.

Comparison with PR: The PR matches this independent proposal exactly. The implementation is clean:

  • join(rig.testDir!, '.qwen-home') → unique per TestRig (each it block gets its own testDir via TestRig.setup())
  • mkdirSync(qwenHome, { recursive: true }) → idempotent, safe
  • env: { ...process.env, QWEN_HOME: qwenHome } → correctly inherits full env, overrides only QWEN_HOME
  • Storage.getGlobalQwenDir() confirmed to honor QWEN_HOME first (storage.ts:155-158)

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 QWEN_HOME mechanism and the same pattern as qwen-config-dir.test.ts. Nothing new to extract.

Testing

This 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):

 ❯ cli/acp-integration.test.ts (11 tests | 1 failed | 9 skipped) 6210ms
   × acp integration > supports session/set_config_option for mode and model 4659ms (retry x2)
     → expected undefined to be defined
   ✓ acp integration > returns error for invalid configId in set_config_option  1549ms

AssertionError: expected undefined to be defined
 ❯ cli/acp-integration.test.ts:516:27
    514|         model.modelId.includes('openai'),
    515|       );
    516|       expect(openaiModel).toBeDefined();

After (PR applied):

 ❯ cli/acp-integration.test.ts (11 tests | 1 failed | 9 skipped) 6157ms
   × acp integration > supports session/set_config_option for mode and model 4620ms (retry x2)
     → expected undefined to be defined
   ✓ acp integration > returns error for invalid configId in set_config_option  1536ms

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 fileParallelism: true (vitest.config.ts:28) — it requires a concurrent test file (e.g. system-control.test.ts) calling setModel() between this test's authenticate and session/new. Running a single file doesn't trigger the race.

The CI job that would validate this (Integration Tests (CLI, No Sandbox)) is gated on merge_group events only (ci.yml:403), so it cannot run on PRs. Validation will happen at merge-queue time.

Agent stderr confirms QWEN_HOME isolation is working:

Warning: QWEN_HOME points to "…/acp-set-config-option/.qwen-home" but no settings.json was found there.

The agent correctly picks up the isolated QWEN_HOME. The single-file failure appears to be a separate issue (the authenticate({ methodId: 'openai' }) call with sk-fake credentials doesn't register an openai model in this environment regardless of isolation).

中文说明

代码审查

独立提案: 给定问题(并行测试文件共享 $HOME/.qwen/settings.json),我会给每个 spawn 的 agent 分配自己的 QWEN_HOME,指向测试临时目录——正是 qwen-config-dir.test.ts 已有的做法。一行 env,一个 mkdirSync,搞定。

与 PR 对比: PR 完全匹配这个独立提案。实现干净:

  • join(rig.testDir!, '.qwen-home') → 每个 TestRig 唯一(每个 it 块通过 TestRig.setup() 获得自己的 testDir
  • mkdirSync(qwenHome, { recursive: true }) → 幂等,安全
  • env: { ...process.env, QWEN_HOME: qwenHome } → 正确继承完整环境,仅覆盖 QWEN_HOME
  • Storage.getGlobalQwenDir() 确认优先读取 QWEN_HOME(storage.ts:155-158)

无正确性问题。无 AGENTS.md 违规。17 行注释块较长但合理——未来维护者调试同一竞态时正需要这些上下文。

测试

这是仅改测试的变更,无用户可见行为,tmux CLI 截图不适用。验证方式是跑受影响的测试。

单文件运行下,PR 前后测试均失败。这符合预期,并不否定 PR。PR 修复的竞态仅在 fileParallelism: true 下显现——需要并发测试文件(如 system-control.test.ts)在本测试的 authenticatesession/new 之间调用 setModel()。单文件运行不会触发竞态。

CI 中验证此修复的 job(Integration Tests (CLI, No Sandbox))仅在 merge_group 事件触发(ci.yml:403),PR 上不会运行。验证将在合入队列时进行。

Agent stderr 确认 QWEN_HOME 隔离生效——agent 正确使用了隔离目录。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Reflection

The PR's approach matches my independent proposal exactly — give each spawned agent its own QWEN_HOME inside the per-test temp directory. Couldn't be simpler.

The race-condition analysis is solid and well-evidenced: shared $HOME/.qwen + fileParallelism: true + authenticate/setModel both writing to User scope = a predictable write-write race between authenticate({ methodId: 'openai' }) and session/new. The fix is the same isolation pattern already used by qwen-config-dir.test.ts, which is strong evidence it's the right approach — the project already solved this exact problem elsewhere.

The code is correct: unique per-test directory, idempotent mkdirSync, proper env inheritance, getGlobalQwenDir() confirmed to honor QWEN_HOME first. One file, 20 lines, no production code touched. If I had to maintain this in six months, I'd thank the author for the detailed comment explaining the race.

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 (Integration Tests (CLI, No Sandbox)) is gated on merge_group. But the theoretical case is strong, the pattern is established, and the change is trivially safe (test-only, isolated env var). Validation will happen at merge-queue time.

Approving. ✅

中文说明

总结

PR 方案与我的独立提案完全一致——给每个 spawn 的 agent 分配测试临时目录下的独立 QWEN_HOME。不能再简单了。

竞态分析扎实且有据:共享 $HOME/.qwen + fileParallelism: true + authenticate/setModel 都写入 User 级 = authenticate({ methodId: 'openai' })session/new 之间可预期的写写竞态。修复采用的是项目已在 qwen-config-dir.test.ts 中使用的隔离模式——项目已在别处解决了同样的问题,这是方案正确的有力证据。

代码正确:每个测试唯一目录、幂等 mkdirSync、正确的环境继承、getGlobalQwenDir() 确认优先读取 QWEN_HOME。一个文件,20 行,零生产代码改动。如果六个月后我来维护,会感谢作者留下的详细竞态注释。

唯一的保留是我无法经验性验证修复——单文件运行不触发并行竞态,CI 验证 job 仅在 merge_group 触发。但理论论证充分,模式已被项目采纳,变更本身安全(仅测试,隔离 env var)。验证将在合入队列时完成。

批准。✅

Qwen Code · qwen3.7-max

@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.

LGTM, looks ready to ship. ✅

@yiliang114 yiliang114 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.

LGTM

@yiliang114
yiliang114 merged commit 6fa1320 into QwenLM:main Jun 23, 2026
27 checks passed
wenshao added a commit that referenced this pull request Jun 23, 2026
… 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.
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.

3 participants