fix(test): pin QWEN_RUNTIME_DIR in daemon integration tests - #7439
Conversation
Session-writer lock files resolve through Storage.getRuntimeBaseDir() which can fall outside the per-test temp HOME in Docker CI, causing cross-test-file lock conflicts that surface as spurious "session is already open in another Qwen process" 409s. Pin QWEN_RUNTIME_DIR alongside QWEN_HOME so locks are fully isolated per test run.
Review:
|
Code Review —
|
|
Thanks for the PR! Template looks good ✓ Problem: observed CI flake with concrete evidence — linked issue #7435 and a specific CI run (29836149258) showing 7 tests failing with Direction: straightforward test-isolation fix, well within scope. The pattern of pinning Size: not applicable (no core paths touched). Approach: minimal and correct. Two lines of env setup (plus a helpful comment in the routes test). Nothing to cut. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 CI flake,有具体证据——关联 issue #7435 和特定 CI 运行(29836149258),显示 7 个测试因 方向:直接的测试隔离修复,完全在范围内。在 规模:不适用(未触及核心路径)。 方案:最小且正确。两行环境变量设置(加上 routes 测试中一个有用的注释)。没有可砍的部分。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: given the Findings: no issues. The diff matches the established pattern in Verified TestingThis is a test-infrastructure fix for a Docker CI flake — no user-visible behavior changes. The meaningful verification is running the affected integration tests locally to confirm no regression: All 39 tests pass with the PR's changes applied. The actual flake fix can only be confirmed by the Docker CI run (the flake doesn't reproduce outside Docker), but the logic is sound: pinning the highest-priority env var guarantees lock files stay inside the isolated temp dir. 中文说明代码审查独立方案: 根据 发现: 无问题。diff 与 已验证 测试这是一个针对 Docker CI flake 的测试基础设施修复——没有用户可见的行为变化。有意义的验证是在本地运行受影响的集成测试以确认无回归: 两个测试文件共 39 个测试全部通过。实际的 flake 修复只能通过 Docker CI 运行来确认(该 flake 在 Docker 外无法复现),但逻辑是可靠的:固定最高优先级的环境变量保证锁文件留在隔离的临时目录内。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — clean, minimal fix for a real CI flake with concrete evidence; matches the established pattern exactly. This is the kind of PR that's easy to evaluate: the problem is real (specific CI run, 7 failing tests, linked issue), the fix is the obvious one (pin the env var that's already pinned in the sibling test file), and the diff is two lines of setup plus a comment explaining why. Nothing to second-guess, nothing to maintain later. The Docker CI run will be the final confirmation, but the logic is airtight — 中文说明置信度:5/5 — 干净、最小的修复,针对有具体证据的真实 CI flake;完全匹配已建立的模式。 这是一个容易评估的 PR:问题是真实的(特定 CI 运行、7 个失败测试、关联 issue),修复是显而易见的(固定已在兄弟测试文件中固定的环境变量),diff 是两行设置加上一个解释原因的注释。没有需要质疑的,没有以后需要维护的。Docker CI 运行将是最终确认,但逻辑是严密的—— — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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_RUNTIME_DIR resolved to the same path as QWEN_HOME, making it a no-op. The real fix is clearing sandboxSessionId in the parent commit.
…wenLM#7443) * fix(acp): clear inherited sandboxSessionId for each new ACP session (QwenLM#7435) Docker sandbox relaunch injects a fixed --sandbox-session-id into the ACP process argv. newSessionConfigInRuntimeContext spreads this.argv into every sub-session config, and config resolution prioritizes sandboxSessionId over sessionId. The first session acquires the writer lease under that fixed ID; every subsequent session reuses the same ID and gets a 409 session_writer_conflict. Clear sandboxSessionId in argvForSession so each newSession() generates its own unique session ID. * revert: remove no-op QWEN_RUNTIME_DIR pin from QwenLM#7439 QWEN_RUNTIME_DIR resolved to the same path as QWEN_HOME, making it a no-op. The real fix is clearing sandboxSessionId in the parent commit.
|
Released in v0.20.1. |
|
PR #7439 (
The triggering event appears to be a stray comment posted by an agent session ("Let me fetch the PR details…") — no action needed. Nothing to change or reply to. ✅ completed |
What this PR does
Pins
QWEN_RUNTIME_DIRalongsideQWEN_HOMEin the daemon env for bothqwen-serve-routes.test.tsandqwen-serve-streaming.test.ts, so session-writer lock files land inside the per-test temp directory instead of resolving throughStorage.getRuntimeBaseDir()which can fall outside the test's isolated HOME in Docker CI.Why it's needed
The Release workflow's Docker integration tests fail intermittently with 7 tests in
qwen-serve-routes.test.tsall hittingDaemonHttpError: POST /session: This session is already open in another Qwen process.(HTTP 409,session_writer_conflict). The No-Sandbox variant passes on the same commit. The session-writer lease mechanism stores lock files underStorage.getRuntimeBaseDir(), whose resolution priority isQWEN_RUNTIME_DIRenv >settings.advanced.runtimeOutputDir>Storage.runtimeBaseDir>QWEN_HOME. The tests setQWEN_HOMEbut notQWEN_RUNTIME_DIR, so in Docker CI the locks can resolve to a shared location where concurrent or prior test runs leave stale lock files with live PIDs, causing spurious conflicts.Reviewer Test Plan
How to verify
Trigger the Release workflow (or the E2E Tests workflow with Docker sandbox) on this branch. The
Integration Tests (Docker)job should pass withoutsession_writer_conflicterrors inqwen-serve-routes.test.ts.Evidence (Before & After)
Before: Run 29836149258 — 7 failed tests, all
session_writer_conflict.After: pending CI run on this branch.
Tested on
Environment (optional)
Docker sandbox CI (the only environment where the flake reproduces).
Risk & Scope
getRuntimeBaseDir()escapesQWEN_HOMEin Docker is not fully traced; this fix sidesteps it by pinning the highest-priority override.Linked Issues
Fixes #7435
中文说明
本 PR 做了什么
在
qwen-serve-routes.test.ts和qwen-serve-streaming.test.ts的 daemon 环境变量中, alongsideQWEN_HOME一起固定QWEN_RUNTIME_DIR,使 session-writer 锁文件落入每个测试的临时目录内,而不是通过Storage.getRuntimeBaseDir()解析到 Docker CI 中测试隔离 HOME 之外的位置。为什么需要
Release 工作流的 Docker 集成测试间歇性失败,
qwen-serve-routes.test.ts中 7 个测试全部报DaemonHttpError: POST /session: This session is already open in another Qwen process.(HTTP 409,session_writer_conflict)。同一 commit 的 No-Sandbox 变体通过。Session-writer lease 机制将锁文件存储在Storage.getRuntimeBaseDir()下,其解析优先级为QWEN_RUNTIME_DIRenv >settings.advanced.runtimeOutputDir>Storage.runtimeBaseDir>QWEN_HOME。测试设置了QWEN_HOME但未设置QWEN_RUNTIME_DIR,因此在 Docker CI 中锁可能解析到共享位置,并发或先前测试运行留下的带有活跃 PID 的过期锁文件会导致虚假冲突。审阅者测试计划
如何验证
在此分支上触发 Release 工作流(或带 Docker sandbox 的 E2E Tests 工作流)。
Integration Tests (Docker)job 应通过,qwen-serve-routes.test.ts中无session_writer_conflict错误。证据(修改前后)
修改前:Run 29836149258 — 7 个测试失败,全部为
session_writer_conflict。修改后:等待此分支的 CI 运行。
测试平台
环境(可选)
Docker sandbox CI(唯一能复现此 flake 的环境)。
风险与范围
getRuntimeBaseDir()在 Docker 中逃逸QWEN_HOME的根本原因未完全追踪;此修复通过固定最高优先级覆盖来绕过。关联 Issue
Fixes #7435