fix(serve): advertise the native directory picker during bootstrap - #10471
Merged
Conversation
#9406 made `native_directory_picker` host-conditional but wired the probe only into the runtime capability path (`createApp` -> `serve-features.ts`). The fast-path bootstrap envelopes, which answer `/capabilities` and `/daemon/status` until the runtime mounts, never set the toggle, so they omitted the tag even on a host whose GUI probe returns true. Two consequences: a client that reads `/capabilities` inside that window hides the workspace Browse affordance on a machine that can open the native picker, and the capabilities-envelope E2E — which lands inside the bootstrap window — fails wherever the probe is true. macOS is the only CI lane where it is, which is why `E2E Test - macOS - shard 2/2` has been red on main since #9406 landed. Probe once while the bootstrap app is built and feed the result to both bootstrap envelopes, mirroring what the runtime path already does at `createApp` time. Probing at app-build time also keeps the `/dev/console` stat and the `PATH` scan for `zenity` off the per-request path.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template: the three main headings are all present ( Problem: an observed bug, not theory — verified against the API. #9406 (merged 2026-08-29) made Direction: aligned — fixes a red Size: not applicable — no core module paths ( Approach: the scope feels right — mirror the probe result into the bootstrap envelope the same way the function already mirrors the WS flags, probed once at app build rather than per request. Two notes: (1) the body says "Fixes the E2E failures tracked in #10450 and #10453", but GitHub's closing parser registers no closing reference through that phrasing, so merging won't auto-close those issues — if the intent is to close them, tighten it to Risk: no high-risk path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板：三个主标题齐全（ 问题：已观测到的 bug，不是理论问题——已通过 API 核实。#9406（2026-08-29 合并）把 方向：对齐——既修复 main 上的红 lane，也修复一个真实的产品缺口（在能打开原生选择器的宿主上，Web Shell 会在 bootstrap 窗口内隐藏 workspace Browse 入口）。这修的是 #9406 本意的行为，无需 CHANGELOG 方向信号。 规模：不适用——未触及核心模块路径（仅 方案：范围合理——把探针结果镜像进 bootstrap envelope，与该函数镜像 WS flags 的既有做法一致，且只在 app 构建时探测一次而非每请求一次。两点提醒：（1）正文写的 "Fixes the E2E failures tracked in #10450 and #10453" 不会被 GitHub 关闭解析器识别为关闭引用，合并后不会自动关闭这两个 issue——如果本意是关闭，建议改成 风险：未命中高风险路径，无升级风险信号。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI wrote my baseline before reading the diff: probe
No blockers, no convention issues. Testing — the PR's own CIMost lanes are still in flight as of this pass; the finalize job will refresh the table when CI settles. One red check, classified as a pre-existing flake, not caused by this PR: Sandboxed verification would settle the remaining claim: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查：读 diff 之前我先写下自己的基线方案——在 bootstrap app 构建时对
无阻塞项，无规范问题。 测试：截至本次检查，PR 自身 CI 大部分仍在运行中，finalize 任务会在 CI 结束后更新表格。唯一红 check 判定为既有 flake，非本 PR 引入： 沙箱验证建议： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean review of a minimal, well-evidenced fix; the only open items are the in-flight CI and the overlap with #10456, which is a human coordination call, not a defect here. My independent proposal for this bug — probe once at bootstrap app build, thread the result into both envelopes the way the runtime path does — is exactly what landed, and I didn't find a simpler path while writing it. The problem was verified before I read a line of code: the first red Reservations, all non-blocking:
Approval deferred until CI lands green on 中文说明我在读代码前写下的独立方案——在 bootstrap app 构建时探测一次，把结果按 runtime 路径的既有方式传入两个 envelope——与 PR 落地的做法完全一致，构思过程中也没找到更简单的路径。问题在读代码之前就已核实：第一个变红的 保留意见，均不阻塞：
审批推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
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. |
qwen-code-ci-bot
approved these changes
Aug 29, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/serve/run-qwen-serve.test.ts — no such file or directory; src/serve/server.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/serve/run-qwen-serve.test.ts — no such file or directory; src/serve/server.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
yiliang114
approved these changes
Aug 29, 2026
This was referenced Aug 29, 2026
This was referenced Aug 29, 2026
qwen-code-dev-bot
added a commit
that referenced
this pull request
Aug 29, 2026
The identical fix already landed on main through #10471 (52e13e4). The conflicts in run-qwen-serve.ts and run-qwen-serve.test.ts are the two PRs' copies of the same change; they resolve to the shipped main version, leaving this branch with no delta against main. The PR is a duplicate of #10471 and can be closed.
Collaborator
|
Released in v0.23.0. |
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
3 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
The fast-path bootstrap envelopes (
GET /capabilitiesand thecapabilities.featuresblock ofGET /daemon/status, both served until the runtime mounts) now advertisenative_directory_picker, probed once while the bootstrap app is built.Why it's needed
#9406 made
native_directory_pickerhost-conditional, but wired the probe only into the runtime path —createApp→createServeFeatures(server.ts/server/serve-features.ts). The bootstrap buildercurrentServeFeaturesForRunQwenServenever set the toggle, so the bootstrap window omitted the tag unconditionally, whatever the host's GUI state.Two consequences:
/capabilitiesinside the bootstrap window hides the workspace Browse affordance (web-shell/client/App.tsxgates on this tag) on a host that can in fact open the native picker. The daemon takes ~1.5s here to swap in the runtime envelope, and the Web Shell reads capabilities on connect, so the window is real, not theoretical.E2E Test - macOS - shard 2/2has been red on everymainpush since feat(serve): hide workspace Browse on headless daemon hosts #9406 landed — first red run is feat(serve): hide workspace Browse on headless daemon hosts #9406's own push run (33223243993), green immediately before it. The capabilities-envelope E2E derives its expectation from the same probe and lands inside the bootstrap window, so the expectation gains a tag the bootstrap envelope can never carry. macOS is the only E2E lane where the probe returns true (Linux runners have noDISPLAY/zenity; there is no Windows E2E lane), which is why the failure is macOS-only.This is the same class of gap the neighbouring comment in that function already guards against for the WS flags ("so the bootstrap
/capabilitieswindow doesn't briefly under-report them").Fixes the E2E failures tracked in #10450 and #10453.
Evidence
The macOS-only failure reproduces on Linux once the host satisfies the probe's Linux branch (
DISPLAYset + an executablezenityonPATH). Same bundle, same test, before the fix:byte-for-byte the macOS CI diff. After the fix:
37 passed (37)with the fake GUI, and37 passed (37)headless (the Linux CI shape), so neither host shape regresses.The bootstrap window is what the E2E reads. Polling a real bundled daemon every 500 ms from
listening(fake GUI host):The 118-feature envelope carries neither
native_directory_pickernordynamic_workspace_registration/scratch_workspace_registration— that is the bootstrap envelope, and 118 is exactly the count CI reports as received. The runtime envelope (125) already advertised the picker correctly, which is why this only ever showed up as a bootstrap-window bug.Unit RED/GREEN. The new
it.each([true, false])case fails onmainwithexpected [ … ] to include 'native_directory_picker'for theavailable: truearm and passes foravailable: false; both arms pass with the fix. It also asserts the probe is not re-run per request, since/dev/consolestat +PATHscan used to sit on the hot path of every bootstrap capabilities/status call.Relation to #10456
#10456 targets the same red lane but reads the cause as probe timing drift — the test re-probing at assertion time while the daemon probed at boot — and moves the test-side probe to daemon spawn time. In the repro above both probes run in the same process, on the same host, seconds apart, and agree; the failure persists, because the bootstrap envelope omits the tag no matter when either side probes. So the test-side change alone leaves the lane red, and the daemon keeps under-reporting the capability to real clients. Happy to close mine if the author prefers to fold this into theirs — the production hunk is what matters.
Reviewer Test Plan
Unit (no build needed):
E2E with a GUI-capable host shape (reproduces the macOS lane on Linux):
Revert the
run-qwen-serve.tshunk and both commands go red on the picker assertion.中文说明
这个 PR 做了什么
fast-path bootstrap 的两个 envelope（runtime 挂载前对外服务的
GET /capabilities和GET /daemon/status里的capabilities.features）现在会带上native_directory_picker，探针在 bootstrap app 构建时只算一次。为什么需要
#9406 把
native_directory_picker改成随宿主 GUI 环境决定，但只接到了 runtime 那条链路（createApp→createServeFeatures）。bootstrap 的构造器currentServeFeaturesForRunQwenServe从来不设这个开关，所以 bootstrap 窗口无论宿主是什么状态都不会带这个 tag。两个后果：
/capabilities的客户端，会在一台其实能弹出原生选择器的机器上把 workspace Browse 入口藏掉（web-shell/client/App.tsx就是按这个 tag 判断的）。本机实测这个窗口约 1.5 秒，而 Web Shell 正是在连接时读 capabilities，所以窗口是真实存在的。main每次 push 的E2E Test - macOS - shard 2/2都是红的 —— 第一次红就是 feat(serve): hide workspace Browse on headless daemon hosts #9406 自己的 push run（33223243993），之前一次是绿的。那条 capabilities E2E 的期望值来自同一个探针，而请求落在 bootstrap 窗口内，于是期望里多出一个 bootstrap envelope 永远给不出的 tag。macOS 是 E2E 里唯一探针为 true 的 lane（Linux runner 没有DISPLAY/zenity，也没有 Windows E2E lane），所以只有 macOS 挂。这跟该函数里已有的那条注释所防的是同一类问题（"so the bootstrap
/capabilitieswindow doesn't briefly under-report them"）。修复 #10450、#10453 追踪的 E2E 失败。
证据
这个"只在 macOS 出现"的失败可以在 Linux 上复现：只要让宿主满足探针的 Linux 分支（设
DISPLAY+PATH上有可执行的zenity）。同一个 bundle、同一个测试，修复前的 diff 与 macOS CI 逐字一致；修复后：假 GUI 下37 passed (37)，无头（Linux CI 形态）下同样37 passed (37)。E2E 读到的确实是 bootstrap envelope：对真实 bundled daemon 从
listening开始每 500ms 轮询一次，+30ms起是 118 个 feature（无 picker / dynamic / scratch），+1599ms起变成 125 个（都有）。118 正是 CI 报告的 received 数量。runtime envelope 本来就正确带了 picker，所以这只是 bootstrap 窗口的漏配。单测 RED/GREEN：新增的
it.each([true, false])在main上available: true那条会挂（to include 'native_directory_picker'），available: false通过；打上修复后两条都过。同时断言探针不会每请求重算 —— 以前/dev/consolestat 和PATH扫描落在每次 bootstrap capabilities/status 请求的热路径上。与 #10456 的关系
#10456 针对的是同一条红 lane，但把原因判成探针的时序漂移（测试在断言时重新探测，daemon 在启动时探测），改法是把测试侧探针挪到 daemon spawn 时。而在上面的复现里，两侧探针在同一进程、同一宿主、相隔几秒、结果一致，失败照旧 —— 因为无论谁在什么时候探测，bootstrap envelope 都不带这个 tag。所以只改测试侧，lane 仍然是红的，daemon 也仍然在对真实客户端少报这个能力。如果作者更愿意把这个改动并进 #10456，我这边关掉也没问题 —— 关键是产品代码那一处。
评审验证步骤
见上方英文命令。把
run-qwen-serve.ts的改动回退掉，两条命令都会在 picker 断言处变红。