fix(test): probe the native directory picker once at daemon spawn time - #10456
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. Approved.
What I verified
Fix logic: isNativeDirectoryPickerAvailable() in the test now runs immediately before daemon = spawn(...) in beforeAll. Both the test-side probe and the daemon's own boot-time probe now execute within the same small time window (process-spawn latency, seconds), rather than the ~9-minute gap that caused the macOS console-state drift. The fix directly closes the mechanism #9406 introduced.
Env alignment: The daemon's spawn env inherits process.env with only 7 QWEN_SERVE_* keys filtered out — none of them are the vars isNativeDirectoryPickerAvailable() reads (SSH_CONNECTION, SSH_TTY on macOS; DISPLAY/WAYLAND_DISPLAY/PATH on Linux). Both calls see the same relevant environment.
Process-identity alignment: On macOS, process.getuid() is the same for test and daemon (same user). statSync('/dev/console').uid is a filesystem read — identical at the same moment between processes.
No exported symbols touched. Pure test change; no API surface, no production code path affected.
CI disclosure
| Job | Status | Note |
|---|---|---|
| Integration Tests (CLI, No Sandbox) | SKIPPING | This job runs the modified file — no direct CI evidence for this PR |
| Test (macos-latest, Node 22.x) | SKIPPING | The platform where the flake manifests — macOS branch depends on CI lane |
| Test (ubuntu-latest, Node 22.x) | PENDING at review time | Standard unit tests |
| Desktop Shell (ubuntu/windows) | pass | Not related to this change |
| Secret scan, CVE audit | pass | Clean |
The two skipped jobs are the platform this PR targets. Per the PR description, the macOS true-branch is intentionally left to the CI macOS lane; the Linux false-branch passes locally 37/37. That is a documented tradeoff, not an oversight. Worth confirming the macOS CI lane runs on merge.
Reviewed with AI assistance.
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献!
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blockers. The fix does exactly what it advertises, and the mechanism checks out against the daemon code:
The author's local Linux run (37/37) exercises only the probe-false branch; that is the author's report, not evidence this pass re-ran (CI runs are the only execution evidence here, per review policy). Testing evidence — PR's own CI at
|
| Check | Conclusion |
|---|---|
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Integration Tests (no-AK, No Sandbox) |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
What CI cannot settle here: the macOS true branch (probe returns true) — the exact branch that flaked — is not verified by this PR's CI, because the lane that flaked never runs on PRs. Sandboxed lanes don't fill this gap either: @qwen-code /verify and @qwen-code /tmux run on Linux and cannot reproduce a macOS GUI-session drift (the same reason the autofix agent stopped). The pre-merge remedy is a maintainer dispatching the E2E Tests workflow (workflow_dispatch) against this branch; otherwise the first post-merge main E2E run is the confirmation, with #10453 already tracking that lane.
中文说明
代码审查
无阻塞项。修复与描述完全一致,且机制经代码核实成立:
- daemon 在启动时固定探测结果:
createServeApp(packages/cli/src/serve/server.ts)中该能力是立即求值的值(deps.nativeDirectoryPickerAvailable ?? isNativeDirectoryPickerAvailable()),而非懒求值的() => ...项——整个生命周期内广播的都是启动时刻的状态。测试在beforeAll中、spawn之前捕获同一探测,正是对这一行为的正确镜像。 - 两侧探测的环境一致:spawn 的 env 只剔除了七个
QWEN_SERVE_*能力开关,并覆盖HOME/QWEN_HOME/模型密钥,这些都不是探测输入——探测读取的是 macOS 的SSH_CONNECTION/SSH_TTY+ 进程/console UID、Windows 的SESSIONNAME、Linux 的DISPLAY/WAYLAND_DISPLAY/PATH+zenity——daemon 子进程全部原样继承。"同主机、同 uid、同相关环境"是事实,而非口号。 - 残余竞争窗口(捕获 → daemon 启动,毫秒级)取代了原先数分钟的漂移窗口,属于结构性消除,而非赌时序。
- 模块级
let nativeDirectoryPickerAtBoot与该文件既有的启动状态写法(homeDir、port、base)一致,新增注释记录了不直观的不变量。无无关改动。
作者本地 Linux 37/37 只覆盖探测为 false 的分支;这是作者的报告,本次审查未独立复跑(按审查规则,执行证据只来自 CI)。
测试证据——该 PR 自身在 67fde72 上的 CI
通过 API 一次性抓取,目前无失败。审查时 Test (ubuntu-latest, Node 22.x) 与 Integration Tests (no-AK, No Sandbox) 仍在运行——上表会在 CI 结束后原地更新。
两个通道是设计上的跳过,与本 PR 无关:ci.yml 的 macOS/Windows 单测通道仅在 merge_group / schedule / workflow_dispatch 时运行;而 flake 所在通道 E2E Test - macOS - shard 2/2 属于 e2e.yml,只在合并后的 main、每晚定时或手动触发时运行,没有 pull_request 触发。
CI 无法覆盖的部分:macOS 真分支(探测返回 true)——恰好是当初 flake 的分支——不会被本 PR 的 CI 验证,因为该通道从不在 PR 上运行。沙箱通道同样无法补齐:@qwen-code /verify 与 @qwen-code /tmux 在 Linux 上运行,无法复现 macOS GUI 会话漂移(也是 autofix 代理停止的原因)。合并前的补救手段是维护者对本分支手动触发 E2E Tests 工作流(workflow_dispatch);否则以合并后首次 main E2E 运行为准,#10453 已在跟踪该通道。
— Qwen Code · qwen3.8-max
Reviewed at 67fde72ab4bd810069b09e9f5e0a46c06159948b · re-run with @qwen-code /triage
|
Confidence: 4/5 — exactly the minimal fix this flake needed, mechanism verified against the daemon code; the one caveat is that the macOS branch it rescues only gets its confirmation post-merge (or via a maintainer-dispatched E2E run). My independent take before reading the diff was: capture the probe once at spawn time and derive the expectation from that — which is precisely what this does, and I found no simpler path. The two things that could have been wrong both check out: the daemon really does pin the probe at boot (the capability is an eagerly-evaluated value in Every line in the diff serves the fix; the added comment records a genuinely non-obvious invariant rather than narrating the code. The residual nit, already flagged in Stage 2: the probe- CI was still running at review time ( 中文说明置信度:4/5 —— 正是这个 flake 所需的最小修复,机制已对照 daemon 代码核实;唯一的保留是它所救的 macOS 分支要到合并后才能得到确认(或维护者手动触发 E2E)。 读 diff 之前我的独立方案就是:在 spawn 时只探测一次,并据此推导期望——与本 PR 完全一致,且我没有找到更简的路径。两个可能出错的地方都核实无误:daemon 确实在启动时固定探测结果(该能力在 diff 中每一行都服务于修复本身;新增注释记录的是一个真正不直观的不变量,而非复述代码。剩余的提示(Stage 2 已说明):真正出过 flake 的探测为 审查时 CI 仍在运行( — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.22.3)
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. ✅
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Verified the alignment premise end-to-end: the macOS probe reads only uid / /dev/console ownership / SSH_CONNECTION / SSH_TTY, none of which the spawn env-strip touches (HOME/QWEN_HOME overrides are irrelevant to it), so the test-process capture and the daemon's boot-time evaluation see identical inputs; the daemon-side value is pinned at envelope construction, so boot-time capture is the right anchor. Residual drift window shrinks from minutes to the sub-second spawn-to-boot gap, honestly disclosed in Risk. Note: the macOS E2E shard is path-filtered off on PR runs, so the true branch will be exercised by main's next E2E run — expected, same surface where the flake was observed.
中文:对齐前提已逐层实证(探针输入在测试进程与 daemon 子进程间完全一致,envelope 在启动时固定),残余竞态窗口收窄且 Risk 段如实声明。CI 无红项。LGTM。
|
I dug into the same red lane while triaging #10450 / #10453 and landed on a different cause — sharing the evidence here so we don't fix past each other. The two probes don't actually drift. The probe's Linux branch only needs What's missing is on the daemon side. 118 is exactly the received count CI reports, and the baseline list in the test carries So probing at spawn time doesn't change the outcome: wherever the host probe is true (macOS CI is the only such lane — Linux runners have no I put the production-side fix up as #10471 — probe once while the bootstrap app is built, feed both bootstrap envelopes ( 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
|
Released in v0.23.0. |
What this PR does
The capabilities-envelope E2E in
qwen-serve-routes.test.tsnow capturesisNativeDirectoryPickerAvailable()once at daemon spawn time and derives the expectednative_directory_pickersplice from that captured value, instead of re-running the probe at assertion time.Why it's needed
Since #9406,
native_directory_pickeris host-conditional: the daemon probes its own GUI session state at boot and advertises accordingly. The test side re-probed at assertion time, minutes later. When the host's GUI/console session state drifts mid-run, the two probes disagree and the exact-equality baseline assertion fails onnative_directory_picker. That is exactly what happened on three consecutive main E2E runs after #9406 landed (commits c13aa35, 045ae1f, 48ec008; tracked in #10450 and #10453, with the autofix bisect pointing at #9406), always onE2E Test - macOS - shard 2/2where this test lives. The autofix agent stopped there because the Linux runner cannot reproduce the macOS lane. Probing at spawn time aligns the test's view with the daemon's boot-time view and removes the drift window.Reviewer Test Plan
How to verify
Run
QWEN_SANDBOX=false npx vitest run --root ./integration-tests cli/qwen-serve-routes.test.ts. All 37 tests pass regardless of whether the host advertises the picker: the expectation is derived from the same probe the daemon evaluates at boot (same host, same uid, same relevant env), so the two can no longer diverge over the course of the run.Evidence (Before & After)
Before: run 33228441400 (main 48ec008),
E2E Test - macOS - shard 2/2—capabilities envelope > advertises all baseline capabilitiesfails: expected 114 features, received 113,native_directory_pickermissing. Same failure in runs 33223243993 and 33226533764. After: locally on Linux (host without GUI, probe false on both sides) the file passes 37/37; the macOS true branch is verified by the CI macOS lane.Tested on
Environment (optional)
QWEN_SANDBOX=false vitest against the bundled CLI.
Risk & Scope
Linked Issues
Fixes #10453. Same failure class as #10450.
中文说明
本 PR 做了什么
qwen-serve-routes.test.ts的 capabilities envelope E2E 现在只在 daemon 启动时探测一次isNativeDirectoryPickerAvailable(),并用这个捕获值决定期望列表中是否拼接native_directory_picker,而不是在断言时再次探测。为什么需要
自 #9406 起,
native_directory_picker按主机 GUI 会话状态动态判定:daemon 在启动时探测并据此决定是否广播。测试侧过去在断言时(数分钟后)重新探测。当主机的 GUI/console 会话状态在运行中途漂移时,两次探测结论不一致,严格相等的基线断言就会在native_directory_picker上失败。这正是 #9406 落地后连续三次 main E2E 运行(提交 c13aa35、045ae1fc、48ec0083,见 #10450 与 #10453,autofix 二分指向 #9406)的失败形态,且始终发生在该测试所在的E2E Test - macOS - shard 2/2。autofix 代理因 Linux runner 无法复现 macOS 通道而停止。把探测提前到启动时刻,使测试与 daemon 的视角一致,消除漂移窗口。评审测试计划
如何验证
运行
QWEN_SANDBOX=false npx vitest run --root ./integration-tests cli/qwen-serve-routes.test.ts。无论主机是否广播该能力,37 个测试全部通过:期望值取自 daemon 启动时评估的同一探测(同主机、同 uid、同相关环境),两侧不会再在运行过程中出现分歧。证据(前后对比)
修复前:运行 33228441400(main 48ec008),
E2E Test - macOS - shard 2/2——capabilities envelope > advertises all baseline capabilities失败:期望 114 项、实收 113 项,缺native_directory_picker;运行 33223243993 与 33226533764 相同。修复后:本地 Linux(无 GUI 主机,两侧探测均为 false)该文件 37/37 通过;macOS 真分支由 CI macOS 通道验证。测试环境
环境(可选)
QWEN_SANDBOX=false vitest,使用打包后的 CLI。
风险与范围
关联 Issue
Fixes #10453。与 #10450 为同一失败形态。