feat(desktop): package Web Shell as a release-ready desktop app - #8132
Conversation
|
The core architecture is validated, but this is not release-ready yet. The remaining work is mostly productization and release engineering rather than another UI implementation. Blocking for a first public beta
Blocking for a stable release, but potentially follow-up work after the first beta
I would keep this PR scoped as the architecture PoC and track the release work as follow-ups under #8092. The shortest path to a real release is a signed and notarized macOS Apple Silicon beta first; Windows, Linux, and automatic updates can then be added as separate, independently verifiable milestones. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Escalating to maintainer — this PR touches auth (cookie-based bootstrap in the serve layer) and release infrastructure (full rewrite of the desktop release workflow), both of which need maintainer sign-off on direction before a full code review.
Template ✓ — all required sections present, bilingual, with a thorough reviewer test plan.
Problem: real and clearly stated — the existing desktop implementation and Web Shell are separate UI surfaces that duplicate feature work. Refs #8092 for the lower-maintenance direction (Web Shell as the shared product surface, native layer limited to packaging/lifecycle/security/updates).
Direction: aligned with #8092. The Tauri shell wraps the existing Web Shell rather than maintaining a second desktop UI. The changes to existing code are minimal — 32 production lines in packages/cli/src/serve/server.ts (cookie bootstrap) and 64 test lines. Everything else is a new, isolated packages/desktop-shell package outside the npm workspace.
Size: ~3,500 production logic lines across 85 files (excluding Cargo.lock at 5,539 lines, package-lock.json at 232 lines, test files at 64 lines, docs at 162 lines, and binary icons). Well above the 1,000-line advisory threshold, but the footprint in existing code is tiny — the bulk is the new isolated package (Rust, bootstrap HTML/JS, build scripts, Tauri config) and the release workflow rewrite.
Approach: coherent for a "release-ready desktop app" — runtime lifecycle management (Rust), startup/recovery UX (bootstrap HTML/JS), authenticated loopback bootstrap (server.ts cookie exchange), release pipeline (desktop-release.yml), and housekeeping (eslint ignores, workspace exclusion, isolation script). Nothing feels extraneous for the stated goal. The cookie bootstrap reuses the existing isDocumentNavigation helper and is gated behind QWEN_CODE_DESKTOP=1.
What needs maintainer eyes:
- The cookie-based auth bootstrap in
server.ts— a new auth path for the desktop shell (token → HttpOnly SameSite=Strict cookie → 303 redirect to token-free URL, then cookie-to-Bearer middleware). - The
desktop-release.ymlrewrite — from Electron/Bun to Tauri/Node.js, with platform-specific signing, notarization, and updater manifest generation. - Whether the
packages/desktop-shellisolation boundary (own lockfiles, excluded from npm workspace) is the right long-term structure.
No high-risk path signals detected. No core module paths touched (packages/core/src/**, auth/providers/models/config/tools/services directories are unchanged).
中文说明
转交 maintainer 审核——此 PR 涉及认证(serve 层中基于 cookie 的 bootstrap)和发布基础设施(桌面发布 workflow 的完整重写),两者都需要 maintainer 在方向上确认后才会进入完整的代码审查。
模板 ✓——所有必填章节齐全,双语,包含详细的 reviewer 测试计划。
问题: 真实且描述清晰——现有桌面端与 Web Shell 是两套独立的 UI 和产品表面,会重复消耗功能开发成本。关联 #8092 中的低维护方向(Web Shell 保持为共享产品表面,原生层只负责打包、生命周期、安全边界、更新和平台集成)。
方向: 与 #8092 对齐。Tauri 壳封装现有 Web Shell,不再维护第二套桌面 UI。对现有代码的改动极小——仅 packages/cli/src/serve/server.ts 中 32 行生产代码(cookie bootstrap)和 64 行测试代码。其余全部是新的、隔离的 packages/desktop-shell 包,位于 npm workspace 之外。
规模: 约 3,500 行生产逻辑代码,涉及 85 个文件(不含 Cargo.lock 5,539 行、package-lock.json 232 行、测试文件 64 行、文档 162 行和二进制图标)。远超 1,000 行大 PR 建议阈值,但对现有代码的影响极小——大部分是新的隔离包(Rust、bootstrap HTML/JS、构建脚本、Tauri 配置)和发布 workflow 重写。
方案: 对于"可发布的桌面应用"目标来说,方案连贯——runtime 生命周期管理(Rust)、启动/恢复体验(bootstrap HTML/JS)、认证 loopback bootstrap(server.ts cookie 交换)、发布流水线(desktop-release.yml)和基础设施维护(eslint 忽略、workspace 排除、隔离脚本)。没有发现与目标无关的多余改动。Cookie bootstrap 复用了现有的 isDocumentNavigation 辅助函数,并且受 QWEN_CODE_DESKTOP=1 环境变量控制。
需要 maintainer 关注的点:
server.ts中基于 cookie 的认证 bootstrap——桌面壳的新认证路径(token → HttpOnly SameSite=Strict cookie → 303 重定向到无 token URL,然后 cookie 转 Bearer 中间件)。desktop-release.yml重写——从 Electron/Bun 迁移到 Tauri/Node.js,包含平台特定的签名、公证和更新清单生成。packages/desktop-shell的隔离边界(独立 lockfile、排除在 npm workspace 之外)是否是正确的长期结构。
未检测到高风险路径信号。未触及核心模块路径(packages/core/src/**、auth/providers/models/config/tools/services 目录均未改动)。
— Qwen Code · qwen3.8-max-preview
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] Author's own beta-readiness checklist (issue comment 5131906450): lists packaging/signing/notarization, packaged-app E2E, startup/failure UX, release CI, and a security review as 'Blocking for a first public beta', but explicitly concludes the PR should stay scoped as an architecture PoC with that work tracked as follow-ups under #8092. This is a scope/productization roadmap requiring maintainer direction sign-off (the triage bot already escalated for auth + release-infra direction), not a code defect this diff can 'fix' — reading the code cannot rule it still-stands or fixed, so it is recorded as cannot-tell.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] Author's own beta-readiness checklist (issue comment 5131906450): lists packaging/signing/notarization, packaged-app E2E, startup/failure UX, release CI, and a security review as 'Blocking for a first public beta', but explicitly concludes the PR should stay scoped as an architecture PoC with that work tracked as follow-ups under #8092. This is a scope/productization roadmap requiring maintainer direction sign-off, not a code defect this diff can 'fix' — reading the code cannot rule it still-stands or fixed, so it is recorded as cannot-tell. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
|
Addressed the bounded follow-up review items in cd1a337:
I did not add debounce logic for high-frequency window events in this PR because that changes desktop event-handling policy beyond this release-readiness fix batch. CI is running on the new head. |
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Unresolved, please confirm: [Critical] Author's own beta-readiness checklist (issue comment 5131906450): lists packaging/signing/notarization, packaged-app E2E, startup/failure UX, release CI, and a security review as 'Blocking for a first public beta', but explicitly concludes the PR should stay scoped as an architecture PoC with that work tracked as follow-ups under #8092. This is a scope/productization roadmap requiring maintainer direction sign-off, not a code defect — reading the code cannot rule it still-stands or fixed, so it is recorded as cannot-tell. Not reviewed: reverse audit of chunks 6, 10-23 (Cargo.lock, package-lock.json) — generated lock files, reverse audit not launched. Not reviewed: 15 of the diff's 29 sections, the invariant check (state, timers, collections) on .github/workflows/desktop-release.yml, the invariant check (counters, return values, error taxonomies) on .github/workflows/desktop-release.yml, the invariant check (config fields, early returns) on .github/workflows/desktop-release.yml — its prompt was built, but no agent on record was launched with it. Not reviewed: verification and reverse audit — each ran and opened its brief, but neither was launched with the prompt the CLI built — the launches were written by hand, so the posted findings cannot be counted as verified, and what the agents were actually asked is not what this skill certifies. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 未决,请确认:[Critical] Author's own beta-readiness checklist (issue comment 5131906450): lists packaging/signing/notarization, packaged-app E2E, startup/failure UX, release CI, and a security review as 'Blocking for a first public beta', but explicitly concludes the PR should stay scoped as an architecture PoC with that work tracked as follow-ups under #8092. This is a scope/productization roadmap requiring maintainer direction sign-off, not a code defect — reading the code cannot rule it still-stands or fixed, so it is recorded as cannot-tell. 未审查:reverse audit of chunks 6, 10-23 (Cargo.lock, package-lock.json) — generated lock files, reverse audit not launched。 未审查:diff 29 个片段中的 15 个、不变量检查(状态、定时器、集合)(.github/workflows/desktop-release.yml)、不变量检查(计数器、返回值、错误分类)(.github/workflows/desktop-release.yml)、不变量检查(配置字段、提前返回)(.github/workflows/desktop-release.yml)——它的 prompt 已构建,但没有任何 agent 有记录用它启动过。 未审查:验证与反向审计——两者都运行并打开了各自的 brief,但都不是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的,发布的发现不能算作已验证,agent 实际被要求做的也不是本 skill 所认证的内容。 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.7-max via Qwen Code /review
|
Pushed What changed:
Verified:
|
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
Local verification reportI built and ran this PR end to end on Linux — real Environment: Debian 13, x86_64, Node 22.22.2, rustc/cargo 1.97.1, The daemon-side auth work is solid and I confirmed it does exactly what the description claims. The native shell has three reproducible blockers that the current test matrix cannot see, because both ✅ What passes
A/B on the new server code — I compiled out each half of the fix independently and re-ran the suite, so the new tests are genuinely load-bearing:
The browser-auth bootstrap does exactly what the description says. Real Chromium against the real bundled daemon, once the runtime app is mounted:
That is a real security improvement: the daemon token no longer passes through JS-reachable storage. Screenshot of the token-free authenticated shell: 06-browser-cookie-bootstrap.png And the packaged app does work when it wins the startup race — this is the real Tauri window, not a browser: 🔴 Blocker 1 — first launch is unusable:
|
| Condition | Launches where the navigation never reached the runtime app |
|---|---|
| idle | 4 / 11 |
| all cores busy | 5 / 5 |
When it loses, the window is a dead end — raw JSON, no bootstrap page, no Retry:
A CI runner is a loaded machine, so I'd expect this to be common there. The gate needs to poll something served by the runtime app (e.g. GET / with navigation headers until it is not 401), not /health.
🟠 Other findings
npm test fails on a fresh checkout. tauri.conf.json declares resources: {"../runtime/qwen-code": …} and tauri_build hard-fails if the path is missing:
thread 'main' panicked at build.rs:5:40:
failed to run Tauri build script: resource path `../runtime/qwen-code` doesn't exist
.gitignore whitelists !runtime/qwen-code/.gitkeep, but that placeholder is not in the PR — git ls-files shows nothing under runtime/. It works in the documented order (build:runtime first) but npm test cannot be a standalone unit-test job; committing the .gitkeep restores the intent.
The Linux Smoke packaged application CI step will always fail. I ran it verbatim — xvfb-run -a npm run smoke:packaged -- src-tauri/target/x86_64-unknown-linux-gnu/release/qwen-code-desktop:
Error: Timed out waiting for packaged desktop runtime.
Runtime info: {"token":"…","url":"http://127.0.0.1:41873/"}
The app was healthy — it wrote runtime-info.json and served the bootstrap redirect. smoke-packaged.js derives the Linux log root from XDG_STATE_HOME, but Tauri's app_log_dir() on Linux resolves under XDG_DATA_HOME / $HOME/.local/share:
looked in: <ws>/state/com.qwen.code.desktop/logs/desktop-runtime.log 0 bytes (created by the script itself)
written to: <ws>/home/.local/share/com.qwen.code.desktop/logs/… 4670 bytes, has "listening on"
The macOS and Windows branches are correct, which is why local macOS verification passed. Simplest fix: set XDG_DATA_HOME alongside XDG_STATE_HOME, or read the log path from the app.
The bundled daemon is not reaped on exit. Closing the window gracefully (WM_DELETE_WINDOW → CloseRequested → app exits):
daemon running before close: 1
app process alive after close: no
daemon after GRACEFUL close: 1 ← still running
daemon after app SIGTERM: 1
RunEvent::Exit/ExitRequested → stop_runtime does not appear to reach the command-group child on Linux, so every launch/close cycle leaks a qwen serve holding a port and a workspace. This contradicts "reaps the daemon on exit" in the description.
The cookie→Authorization bridge is not gated on the desktop shell. The minting side is correctly gated, and the test names say so — but the accepting side (server.ts:1248) is mounted unconditionally, so every qwen serve now treats a qwen-daemon-token cookie as a bearer. The origin wall makes this hard to abuse and I could not turn it into an exploit, but SameSite=Strict does not separate 127.0.0.1:<other-port> from the daemon port (for an IP host, the "site" ignores the port), and no-Origin sub-resource GETs would carry the cookie. Gating that middleware on desktopShellBootstrap too would keep the blast radius where the tests imply it is.
Doc drift. packages/desktop-shell/README.md says the QWEN_DESKTOP_WORKSPACE "default is the process working directory", but initial_workspace() returns None and emits workspace-required.
Summary
The daemon-side change is correct, well-tested and A/B-verified; I'd merge that part as-is. The native shell needs blockers 1–3 fixed before this can ship as release-ready — 1 and 2 are two config lines, 3 is a readiness-gate change. I'd also fix the Linux smoke log path, since that job will otherwise fail on the first dry_run=true dispatch.
中文版
本地验证报告
我在 Linux 上把这个 PR 完整跑通了:真实的 cargo 工具链、真实的 npm run build:runtime、真实的 Tauri release 二进制、真实的 bundled qwen serve、Xvfb 下真实的 WebKitGTK 窗口,以及真实 Chromium 做浏览器层的 A/B。
环境: Debian 13、x86_64、Node 22.22.2、rustc/cargo 1.97.1、@tauri-apps/cli 2.11.4、libwebkit2gtk-4.1 2.52.5。worktree 在 1f85910(PR head)。
daemon 侧的认证改动很扎实,我确认它完全符合描述。原生壳有 3 个可稳定复现的阻断问题,现有测试矩阵看不到它们,因为 smoke:packaged 和文档里的手动流程都设置了 QWEN_DESKTOP_WORKSPACE,这会完全跳过 bootstrap UI。
✅ 通过的部分
| 检查项 | 结果 |
|---|---|
packages/cli server.test.ts |
841/841 通过(含 4 个新的 browser-bootstrap 测试) |
run-qwen-serve.test.ts 新增 desktop-marker 测试 |
通过 |
process-env-guard.test.ts |
通过 |
packages/desktop-shell npm test(cargo test) |
17/17 通过 |
cargo clippy --all-targets -- -D warnings |
干净 |
npx eslint(所有新增 JS + 改动的 serve 文件) |
干净 |
npm run test:release / npm run smoke:runtime |
通过 |
node scripts/check-desktop-isolation.js |
通过 |
npm run tauri build --target x86_64-unknown-linux-gnu |
构建成功(--no-sign 在 CLI 2.11.4 里确实存在,我核对过) |
对新 server 代码做了 A/B:分别把修复的两半编译掉再跑测试,证明新测试确实绑定在改动上。
| 改动 | 结果 |
|---|---|
if (webShellDir && opts.token && desktopShellBootstrap) → if (false && …) |
2 个测试变红:expected 200 to be 303 |
关掉 cookie→Authorization 桥接 |
1 个测试变红:expected 401 to be 200 |
| 不改 | 全绿 |
浏览器认证 bootstrap 的行为和描述完全一致(真实 Chromium + 真实 bundled daemon,在 runtime app 挂载之后):
| 观测点 | 无修复(gate 编译掉) | 有修复 |
|---|---|---|
GET /?token=…(导航) |
200 |
303 → / |
| 地址栏 URL | /(JS 跑完后客户端剥离) |
/(token 从未出现过) |
Set-Cookie |
无 | qwen-daemon-token、HttpOnly、SameSite=Strict |
| 页面 JS 能否拿到 token | 能 — sessionStorage["qwen-daemon-token"] |
不能 |
页面内无 Authorization 的 fetch('/capabilities') |
401 |
200 |
| Web Shell API 调用 | 19 × 200 |
19 × 200 |
这是实打实的安全提升:daemon token 不再经过 JS 可读的存储。
打包应用在赢下启动竞态时确实能正常工作(下面是真实 Tauri 窗口,不是浏览器):01-desktop-web-shell-ok.png
🔴 阻断 1 — 首次启动完全不可用:window.__TAURI__ 从未注入
bootstrap/bootstrap.js 第 1 行读 window.__TAURI__,但 tauri.conf.json 没有设置 app.withGlobalTauri,而 tauri-utils 的默认值是 false(tauri-utils-2.9.3/src/config.rs:4470)。于是 invoke 和 listen 都是 undefined,initialize() 走了提前返回分支。
任何没有已保存 workspace 的启动(也就是每一次真正的首次运行)都会停在这里。Choose workspace、Retry、Open logs 都渲染出来了但全是死的,用户没有任何前进路径:03-bridge-unavailable.png
现有测试看不到,是因为 scripts/smoke-packaged.js 设了 QWEN_DESKTOP_WORKSPACE,initial_workspace() 直接短路,窗口径直导航到 Web Shell。
🔴 阻断 2 — src-tauri/capabilities/ 是空的,所有 IPC 被 ACL 拒绝
这个藏在阻断 1 后面。我把 withGlobalTauri 设为 true 重新构建后,bridge 有了,紧接着失败的就是 ACL —— capabilities/ 里只有 .gitkeep,且 tauri.conf.json 写的是 "capabilities": []:
Command plugin:event|listen not allowed by ACL
最小修复(已验证):加上 withGlobalTauri: true,再加一个 src-tauri/capabilities/default.json:
{
"identifier": "default",
"description": "Desktop bootstrap window IPC",
"windows": ["main"],
"permissions": ["core:default", "dialog:default", "opener:default", "updater:default"]
}就能渲染出这个 PR 本来想要的页面 —— 绿色脉冲、页脚 Desktop 0.0.1、Retry 正确隐藏:05-bootstrap-after-minimal-fix.png
这两处改动我事后都已回滚,报告里其余所有数据都来自未修改的 PR。
🔴 阻断 3 — 就绪门禁并没有守住窗口真正要导航的目标
runtime.rs::wait_for_health() 等 GET /health + bearer 返回 {"status":"ok"},然后 start_runtime_async 立刻 window.navigate(<base>/?token=…)。
但 /health 是由 bootstrap app 提供的(isBootstrapServeRoute),而且在 deferred-runtime 路径下,正是这第一次 health 请求触发了 runtime 的加载:
09:03:29.182 deferred runtime: health served, scheduling start in 50ms ← 门禁在这里通过
09:03:29.233 deferred runtime: health timer fired, starting
09:03:29.520 route=GET / status=303 ← webview 在这里才到
这中间的窗口里,createDelegatingServeApp 会在 await runtimeReady 之前同步跑 authenticateDeferredRuntimeRequest = bearerAuth(opts.token)。文档导航不带 Authorization 头,于是直接吃一个 401 —— 而 pre-auth 的 Web Shell 挂载和本 PR 新增的 bootstrap 重定向中间件都在 runtime app 里面,两者都无法应答。
在 bundled daemon 上实测:门禁通过后 shell 有 140–189 ms 不可达(5/5 次)。应用大约在门禁后 270–340 ms 才导航,余量只有约 150 ms,而且经常输:
| 条件 | 导航从未到达 runtime app 的启动次数 |
|---|---|
| 空闲 | 4 / 11 |
| 所有核心跑满 | 5 / 5 |
输掉时窗口就是死路 —— 裸 JSON,没有 bootstrap 页,没有 Retry:02-race-unauthorized.png
CI runner 就是负载很高的机器,我预计那边会经常触发。门禁应该轮询由 runtime app 提供的东西(例如带导航头的 GET / 直到不是 401),而不是 /health。
🟠 其他发现
全新 checkout 上 npm test 会失败。 tauri.conf.json 声明了 resources: {"../runtime/qwen-code": …},而 tauri_build 在路径缺失时硬失败:
thread 'main' panicked at build.rs:5:40:
failed to run Tauri build script: resource path `../runtime/qwen-code` doesn't exist
.gitignore 里白名单了 !runtime/qwen-code/.gitkeep,但这个占位文件并不在 PR 里 —— git ls-files 在 runtime/ 下什么都没有。按文档顺序(先 build:runtime)是能跑的,但 npm test 就无法作为独立的单测 job;把 .gitkeep 提交上来即可恢复原本意图。
Linux 的 Smoke packaged application CI 步骤必定失败。 我逐字跑了一遍 xvfb-run -a npm run smoke:packaged -- src-tauri/target/x86_64-unknown-linux-gnu/release/qwen-code-desktop:
Error: Timed out waiting for packaged desktop runtime.
Runtime info: {"token":"…","url":"http://127.0.0.1:41873/"}
应用本身是健康的 —— 它写出了 runtime-info.json,也正常提供了 bootstrap 重定向。问题是 smoke-packaged.js 用 XDG_STATE_HOME 推导 Linux 日志根目录,而 Tauri 的 app_log_dir() 在 Linux 上解析到 XDG_DATA_HOME / $HOME/.local/share:
脚本查找: <ws>/state/com.qwen.code.desktop/logs/desktop-runtime.log 0 字节(脚本自己创建的)
实际写入: <ws>/home/.local/share/com.qwen.code.desktop/logs/… 4670 字节,含 "listening on"
macOS 和 Windows 分支是对的,这也是本地 macOS 验证能通过的原因。最简单的修法:在设 XDG_STATE_HOME 的同时也设 XDG_DATA_HOME,或者直接从应用读日志路径。
退出时 bundled daemon 没有被回收。 优雅关闭窗口(WM_DELETE_WINDOW → CloseRequested → 应用退出):
关闭前 daemon: 1
关闭后应用进程存活: 否
优雅关闭后 daemon: 1 ← 仍在运行
应用 SIGTERM 后: 1
RunEvent::Exit/ExitRequested → stop_runtime 在 Linux 上似乎没能触达 command-group 子进程,因此每次「启动—关闭」都会泄漏一个占着端口和 workspace 的 qwen serve。这与描述中的 "reaps the daemon on exit" 不符。
cookie→Authorization 桥接没有按 desktop shell 收敛。 签发侧的 gate 是对的,测试名也这么写 —— 但接受侧(server.ts:1248)是无条件挂载的,于是所有 qwen serve 现在都会把 qwen-daemon-token cookie 当作 bearer。origin 墙让它很难被利用,我也没能构造出可用的攻击;但 SameSite=Strict 并不能把 127.0.0.1:<其他端口> 和 daemon 端口区分开(对 IP 主机来说 "site" 忽略端口),而不带 Origin 的子资源 GET 是会携带该 cookie 的。把这个中间件也按 desktopShellBootstrap 收敛,能让影响面和测试暗示的范围一致。
文档不一致。 packages/desktop-shell/README.md 说 QWEN_DESKTOP_WORKSPACE 的「默认是进程工作目录」,但 initial_workspace() 返回 None 并发出 workspace-required。
结论
daemon 侧的改动是正确的、测试充分且经过 A/B 验证,这部分我认为可以直接合入。原生壳需要先修掉阻断 1–3 才谈得上 release-ready —— 1 和 2 是两行配置,3 是就绪门禁的改法。另外建议一并修掉 Linux smoke 的日志路径,否则第一次 dry_run=true 派发就会失败。
- Cookie→Bearer translation middleware now only active when desktopShellBootstrap is enabled - Use timing-safe comparison for bootstrap token validation
Re-verification at
|
desktopShellBootstrap |
GET /health with Cookie: qwen-daemon-token=… |
|
|---|---|---|
false — plain qwen serve |
401 |
✅ fixed |
false, gate reverted to pre-fix |
200 |
← the old behaviour |
true — desktop shell |
200 |
still works |
The blast radius is now exactly what the test names imply. Worth noting the PR ships no test for this — nothing in server.test.ts fails if the gate is removed again. A one-liner asserting 401 for desktopShellBootstrap: false + a cookie would lock it in.
All suites still green on the new head: server.test.ts + run-qwen-serve.test.ts + process-env-guard.test.ts = 1052/1052; cargo test 17/17; clippy clean; eslint clean; test:release, smoke:runtime, check-desktop-isolation all pass.
🔴 New regression from the same commit: ?token= with a multi-byte character returns 500
The timing-safe comparison compares JS string length but then hands the strings to timingSafeEqual as UTF-8 buffers:
queryToken.length === token.length &&
timingSafeEqual(Buffer.from(queryToken), Buffer.from(token))'a'.repeat(63) + '中' has .length === 64 (same as the 64-hex-char daemon token) but 66 UTF-8 bytes, so timingSafeEqual throws RangeError: Input buffers must have the same byte length. Express turns that into a 500.
Live, against the rebuilt bundled daemon (QWEN_CODE_DESKTOP=1, --require-auth):
correct token (expect 303) -> 303
wrong ASCII, same length (expect 200) -> 200
same JS length, 66 utf8 bytes -> 500 ← was 200 before f3f1ee5f
short wrong token (expect 200) -> 200
daemon log: qwen serve: unhandled error: RangeError: Input buffers must have the same byte length
route=GET / durationMs=8 status=500 request completed
To be fair on severity: the daemon does not crash and the next request serves the shell normally, so this is an isolated 500 on a crafted navigation, not a DoS. But it is a behaviour change on the exact path the commit was hardening, and it is trivially avoidable — compare Buffer.byteLength, or hash both sides first the way bearerAuth already does in auth.ts:
const expected = createHash('sha256').update(token, 'utf8').digest();
// …then timingSafeEqual over two fixed-width 32-byte digestsThat also removes the length-equality precondition, which is itself a (minor) length oracle.
🔴 Still open — all three blockers reproduce on the new build
packages/desktop-shell/ is byte-identical to the commit I tested before, so this is expected; re-confirmed empirically rather than inferred:
| Finding | Status at f3f1ee5f |
|---|---|
1. withGlobalTauri unset → window.__TAURI__ undefined → first launch shows "Desktop bridge unavailable" |
reproduces — screenshot |
2. empty src-tauri/capabilities/ → Command plugin:event|listen not allowed by ACL |
reproduces |
3. /health readiness gate doesn't cover the runtime-app mount |
reproduces — 5/5 launches under CPU load |
Linux Smoke packaged application CI step |
still fails: Error: Timed out waiting for packaged desktop runtime |
npm test on a fresh checkout (missing runtime/qwen-code/.gitkeep) |
still fails |
| bundled daemon not reaped on exit | still leaks — daemon after GRACEFUL close: 1 |
One honest update on blocker 3: the timing margin shifted with the main merge. On an idle machine I now get 0/8 hits (was 4/11 at 1f85910), so it is less likely to bite during casual local testing. Under load it is still 5/5, unchanged. The structural problem is identical — /health is answered by the bootstrap app and that first request is what triggers the deferred runtime load, so the gate returns ~150 ms before / stops returning 401. A CI runner or a cold machine will land in that window; the improved idle number is luck, not a fix.
Summary
f3f1ee5f correctly closes the cookie-scope issue — nice fix, just needs a regression test and a Buffer.byteLength (or hash-first) correction for the multi-byte 500. The three native-shell blockers from my previous report are still open and still reproduce; 1 and 2 are two config lines, 3 needs the readiness gate to poll something the runtime app actually serves.
中文版
在 f3f1ee5f 上的复验
在更新后的 head 上重跑了完整的本地构建 —— 全新 npm ci、全新 npm run build:runtime(bundle 现在记录 qwenCodeCommit f3f1ee5f、qwenCodeVersion 0.21.2),Linux/Tauri/Xvfb 环境同上次。
自上次报告以来只有 一个实质性提交(f3f1ee5f)加一次 main 合并。git diff 1f85910b f3f1ee5f -- packages/desktop-shell .github/workflows/desktop-release.yml .github/scripts/ 是 空的 —— 也就是说三个阻断问题和 CI/日志问题都没有动。不过我还是在新构建上重新实测了一遍,而不是靠推断。
✅ 已修复并验证:cookie 收敛
关于 cookie→Authorization 桥接无条件挂载的 🟠 问题已经正确修复,而且确实起作用。我把它还原成修复前的形态(if (desktopShellBootstrap) → if (true))做了 A/B:
desktopShellBootstrap |
带 Cookie: qwen-daemon-token=… 的 GET /health |
|
|---|---|---|
false —— 普通 qwen serve |
401 |
✅ 已修复 |
false,把 gate 还原成修复前 |
200 |
← 旧行为 |
true —— desktop shell |
200 |
仍然可用 |
影响面现在和测试名暗示的范围一致了。但要指出:这个 PR 没有为此加任何测试 —— 把 gate 再去掉,server.test.ts 里不会有任何一条变红。加一行「desktopShellBootstrap: false + cookie 应得 401」就能锁住。
新 head 上所有套件依然全绿:server.test.ts + run-qwen-serve.test.ts + process-env-guard.test.ts = 1052/1052;cargo test 17/17;clippy 干净;eslint 干净;test:release、smoke:runtime、check-desktop-isolation 全部通过。
🔴 同一个提交引入的新回归:?token= 含多字节字符会返回 500
timing-safe 比较用的是 JS 字符串长度,但传给 timingSafeEqual 的是 UTF-8 buffer:
queryToken.length === token.length &&
timingSafeEqual(Buffer.from(queryToken), Buffer.from(token))'a'.repeat(63) + '中' 的 .length === 64(和 64 位 hex 的 daemon token 相同),但 UTF-8 是 66 字节,于是 timingSafeEqual 抛出 RangeError: Input buffers must have the same byte length,Express 把它变成 500。
对重新构建的 bundled daemon 实测(QWEN_CODE_DESKTOP=1、--require-auth):
正确 token(预期 303) -> 303
错误 ASCII、等长(预期 200) -> 200
JS 长度相同、UTF-8 66 字节 -> 500 ← f3f1ee5f 之前是 200
较短的错误 token(预期 200) -> 200
daemon 日志: qwen serve: unhandled error: RangeError: Input buffers must have the same byte length
route=GET / durationMs=8 status=500 request completed
严重程度上要说公道话:daemon 不会崩溃,下一个请求照常返回 shell,所以这只是构造导航上的一次孤立 500,不是 DoS。但它恰好发生在这个提交想要加固的那条路径上,而且很容易避免 —— 比较 Buffer.byteLength,或者像 auth.ts 里 bearerAuth 已有的做法先做哈希:
const expected = createHash('sha256').update(token, 'utf8').digest();
// …然后对两个定长 32 字节摘要做 timingSafeEqual这样也顺带去掉了长度相等这个前置条件本身构成的(轻微的)长度 oracle。
🔴 仍未解决 —— 三个阻断问题在新构建上照样复现
packages/desktop-shell/ 与我上次测试的提交逐字节相同,所以这在预期之内;下面是实测复现,不是推断:
| 问题 | 在 f3f1ee5f 的状态 |
|---|---|
1. 未设 withGlobalTauri → window.__TAURI__ undefined → 首次启动显示 "Desktop bridge unavailable" |
复现 —— 截图 |
2. src-tauri/capabilities/ 为空 → Command plugin:event|listen not allowed by ACL |
复现 |
3. /health 就绪门禁没有覆盖 runtime app 的挂载 |
复现 —— 满载下 5/5 次启动命中 |
Linux Smoke packaged application CI 步骤 |
依然失败:Error: Timed out waiting for packaged desktop runtime |
全新 checkout 上的 npm test(缺 runtime/qwen-code/.gitkeep) |
依然失败 |
| 退出时 bundled daemon 未回收 | 依然泄漏 —— daemon after GRACEFUL close: 1 |
关于阻断 3 有一个需要如实更新的点:main 合并之后时序余量变了。在空闲机器上我现在是 0/8 命中(在 1f85910 上是 4/11),所以本地随手测试时更不容易碰到。但满载下依然是 5/5,没有变化。结构性问题完全一样 —— /health 由 bootstrap app 应答,而这第一次请求正是触发 deferred runtime 加载的动作,所以门禁比 / 停止返回 401 早约 150 ms。CI runner 或冷启动的机器会落进这个窗口;空闲数字变好是运气,不是修复。
结论
f3f1ee5f 正确收敛了 cookie 的作用域 —— 修得不错,只差一个回归测试,以及把多字节 500 用 Buffer.byteLength(或先哈希)修掉。上一份报告里的三个原生壳阻断问题仍然存在且照样复现;其中 1 和 2 是两行配置,3 需要让就绪门禁去轮询 runtime app 真正提供的东西。
|
🤖 Addressed the latest review feedback (round 15/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 15/100 轮)。改动内容与我反驳保留之处如下: Addressed feedback for PR #8132Critical fixes (3)1. Verified: Fix: added 2. Verified: Fix: replaced the single fetch with a retry loop (500 ms interval, 30 s deadline) that waits for the fallback timer to start the runtime and the build to finish. The loop also checks 3. Verified: the clobber branch ran Fix: added Deferred suggestions (7) — critical-only mode after 10 roundsPer AGENTS.md's review-round policy ("once a PR has been through roughly 5 review rounds, land only Critical fixes"), the following automated-reviewer suggestions are deferred to a follow-up:
Conflict notesNo conflicts ( Verification
中文说明PR #8132 反馈处理Critical 修复(3 项)1. 已验证: 修复:在浅层 health 探测与未授权 shell 断言之间加入 2. 已验证: 修复:将单次请求替换为重试循环(500 ms 间隔,30 s 超时),等待兜底定时器启动 runtime 并完成构建。循环内同时检查 3. 已验证:clobber 分支执行 修复:在 clobber 上传后立即添加 延后的建议(7 项)— 10 轮后进入仅处理 Critical 模式根据 AGENTS.md 的审查轮次策略("PR 经过约 5 轮审查后,仅处理 Critical 修复"),以下自动审查建议延后至后续 PR:
冲突说明无冲突( 验证
Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] Maintainer release-readiness sign-off (issue-level comment by @yiliang114): the PR author states 'this is not release-ready yet' and lists productization work (packaged-app E2E coverage, etc.) as 'Blocking for a first public beta'. This is a product/release sign-off that cannot be ruled fixed on code grounds — requires a maintainer decision. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未决,请确认:[Critical] Maintainer release-readiness sign-off (issue-level comment by @yiliang114): the PR author states 'this is not release-ready yet' and lists productization work (packaged-app E2E coverage, etc.) as 'Blocking for a first public beta'. This is a product/release sign-off that cannot be ruled fixed on code grounds — requires a maintainer decision. 未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
| } else { | ||
| return Err(format!("Failed to replace desktop settings: {error}")); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Every error path in write_atomic after fs::write(&temporary, …) succeeds returns Err without removing the temp file, orphaning one desktop-state.json.N.tmp per failed save. — Concrete cost: on the Windows file-lock/AV path this fallback exists for (or a ?-propagated backup-rename failure), repeated save failures accumulate unbounded stale .tmp files in the app config directory; best-effort clean up before each error return.
| } else { | |
| return Err(format!("Failed to replace desktop settings: {error}")); | |
| } | |
| } else { | |
| let _ = fs::remove_file(&temporary); | |
| return Err(format!("Failed to replace desktop settings: {error}")); | |
| } |
中文说明
write_atomic 中 fs::write(&temporary, …) 成功之后的所有错误路径都直接返回 Err,却没有删除临时文件,导致每次保存失败都会在配置目录遗留一个 desktop-state.json.N.tmp。— 具体代价:在本兜底逻辑所针对的 Windows 文件锁/杀毒软件场景(或经 ? 传播的备份重命名失败)下,反复保存失败会在应用配置目录无限累积陈旧的 .tmp 文件;建议在每个错误返回前尽力清理临时文件。
— qwen3.8-max-preview via Qwen Code /review
| cwd: packageDir, | ||
| env: { ...process.env, QWEN_SERVER_TOKEN: token }, | ||
| stdio: ['ignore', 'pipe', 'pipe'], |
There was a problem hiding this comment.
[Suggestion] smoke-runtime.js spawns the bundled daemon against the invoking user's real profile — no HOME/LOCALAPPDATA/XDG_STATE_HOME/XDG_DATA_HOME isolation — unlike smoke-packaged.js, which isolates all four plus a temp workspace. — Concrete cost: every local npm run smoke:runtime writes daemon/daemon.log and debug files into the real ~/.qwen, loads the user's real settings/auth (which can change daemon behavior or crash the smoke for reasons unrelated to the bundle under test), and --workspace packageDir can drop workspace state into the source tree. Mirror smoke-packaged.js's mkdtempSync-based isolation.
中文说明
smoke-runtime.js 以调用者的真实用户配置启动打包 daemon——没有隔离 HOME/LOCALAPPDATA/XDG_STATE_HOME/XDG_DATA_HOME——而 smoke-packaged.js 对这四者及临时 workspace 都做了隔离。— 具体代价:每次本地运行 npm run smoke:runtime 都会把 daemon/daemon.log 和调试文件写入真实的 ~/.qwen,加载用户真实的 settings/auth(可能改变 daemon 行为,或因与被测 bundle 无关的原因使 smoke 崩溃),且 --workspace packageDir 可能把 workspace 状态写入源码树。建议参照 smoke-packaged.js 基于 mkdtempSync 的隔离方式。
— qwen3.8-max-preview via Qwen Code /review
| fn focus_main_window(app: &AppHandle) { | ||
| if let Some(window) = app.get_webview_window("main") { | ||
| let _ = window.unminimize(); |
There was a problem hiding this comment.
[Suggestion] On macOS, closing the main window leaves the app alive with no window, and nothing recreates it: focus_main_window silently no-ops when get_webview_window("main") returns None, and the run closure has no RunEvent::Reopen arm. — Concrete cost: after a macOS user closes the window, clicking the Dock icon (Reopen falls into _ => {}) or relaunching (single-instance routes to focus_main_window → None) never shows a window; the qwen serve daemon keeps running with no UI attached and the user must force-quit. (Windows/Linux exit on last-window-close, so they are unaffected.) Recreate/re-show the window when it is missing (e.g. build it via the same WebviewWindowBuilder path when None), and/or handle RunEvent::Reopen on macOS.
中文说明
在 macOS 上,关闭主窗口后应用仍存活但没有窗口,且没有任何路径重建窗口:当 get_webview_window("main") 返回 None 时 focus_main_window 会静默地什么都不做,而 run 闭包没有 RunEvent::Reopen 分支。— 具体代价:macOS 用户关闭窗口后,点击 Dock 图标(Reopen 落入 _ => {})或重新启动(单实例插件路由到 focus_main_window → None)都不会再显示窗口;qwen serve daemon 继续运行却没有 UI,用户只能强制退出。(Windows/Linux 在最后一个窗口关闭时退出,因此不受影响。)建议在窗口缺失时重建/重新显示窗口(例如当为 None 时用相同的 WebviewWindowBuilder 路径构建),并/或在 macOS 上处理 RunEvent::Reopen。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round summary — no code changesMode: Critical-only (after 10 change-producing rounds) Feedback triage
Why no code changesThe sole Critical finding asks for a maintainer release-readiness decision. The PR author has explicitly listed remaining productization work as blocking. Whether to merge before, after, or independently of that work is a scope and product judgment that requires human maintainer input — it is not a defect fixable in code. Open question for maintainerThe PR author (@yiliang114) states this is "not release-ready yet" and lists blocking items for a first public beta (packaged-app E2E coverage, etc.). A maintainer should decide:
No recommendation is made here as this is a product-direction call. 中文说明Autofix 轮次总结 — 无代码变更模式: 仅处理 Critical(已完成 10 个产生改动的轮次) 反馈分类
为何无代码变更唯一的 Critical 发现要求维护者做出发布就绪决策。PR 作者已明确列出剩余产品化工作作为阻断项。是否在这些工作完成之前、之后或独立于这些工作进行合并,是需要人工维护者判断的范围和产品决策——这不是代码可修复的缺陷。 待维护者决定的问题PR 作者(@yiliang114)声明"尚未达到发布就绪状态",并列出了首次公开测试版的阻断项(打包应用 E2E 覆盖等)。维护者需要决定:
此处不做推荐,因为这是产品方向决策。 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 46 passed · 0 failed · 46 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:46 通过 · 0 失败 · 46 总计 Verification reportPR #8132 Deep Verification —
|
| Cell | Environment | Oracle | Result |
|---|---|---|---|
| A — head | real daemon, bearer-only (auth.ts unmodified) |
new test ignores daemon cookies entirely… |
green, vitest exit 0 (1 passed | 849 skipped) |
| B — control | same, + mutation: accept qwen-daemon-token cookie |
same test | red, AssertionError: expected 200 to be 401 |
The guard test flips green→red exactly when cookie auth is (re)introduced, while the positive control accepts the right token stays green under the same mutation (auth not globally broken) — so the test pins the bearer-only property at the intended assertion. Head 1/1 pass vs control 0/1. Witness: 01-serve-auth-ab.png.
A/B 2 — the fragment-auth integration chain (verified against real code, all three layers)
The PR wires into pre-existing, tested infrastructure rather than changing it:
| Layer | What it does | Evidence |
|---|---|---|
Native (Rust runtime.rs) |
32-byte random token → QWEN_SERVER_TOKEN env; spawns serve --port 0 --hostname 127.0.0.1 --require-auth --no-open; polls health?deep=true with the bearer until body contains "status":"ok"; navigates to base_url/#token=<token> (set_fragment) |
Read-review; Rust unit test carries_the_daemon_token_only_in_the_url_fragment present (not run — no cargo) |
Daemon (serve) |
bearerAuth only; isDeepHealthQuery accepts "true"; bootstrap deep health returns 503 {status:"degraded",reason:"bootstrap"} (no "status":"ok"), so the Rust poll cannot pass before the runtime mounts — the startup-race avoidance the PR claims is real |
health-query.ts, run-qwen-serve.ts:1421 read; unchanged by PR |
Client (web-shell config/daemon.ts) |
reads #token= from location.hash, prefers fragment over query, strips it via removeDaemonTokenFromUrl() |
daemon.test.ts 19/19 pass (ran) |
The token never reaches the server in an HTTP request (fragment semantics); API routes stay bearer-gated. The PR's own smoke-packaged.js encodes exactly this contract (see Correction below).
Corrections
The PR description misdescribes the authentication mechanism. Both the English "After" section and the 中文说明 state the installed app "exchanges the one-shot URL token for an HttpOnly SameSite=Strict cookie, redirects to a token-free URL." That was an earlier design that the PR itself replaced — commit fix(desktop): replace cookie handshake with URL fragment auth. The final code uses URL-fragment token → Authorization: Bearer header, with no cookie at all:
- Rust:
runtime.rs:128url.set_fragment(Some(&format!("token={}", self.token))); noSet-Cookieanywhere insrc-tauri/src,bootstrap/, orscripts/. - Daemon:
bearerAuthreads only theAuthorizationheader; the new serve test asserts aqwen-daemon-tokencookie yields 401. - The PR's own
smoke-packaged.js:142-145throws'Packaged desktop Web Shell must not mint auth cookies'ifgetSetCookie().length > 0, and its comment states "the token travels in the URL fragment, which never reaches the server."
This is labeled a correction to the description, not a request to change code: the code is correct and self-consistent; only the prose is stale. A reviewer reading the description would otherwise form a wrong model of the security boundary (cookie-based vs fragment-based).
Findings
Ordered by severity. Neither is blocking.
1. (Low / documentation) Stale auth description — see Correction above
The description's cookie claim is contradicted by the shipped code and the PR's own smoke assertion. Suggested action: update the "After"/中文说明 paragraphs to describe the fragment-token → bearer exchange. No code change.
2. (Informational / nit) Update-manifest interpolates the release tag raw
.github/scripts/create-desktop-update-manifest.mjs percent-encodes the artifact filename (encodeURIComponent(artifact)) but interpolates options.tag directly into the download URL. Feeding a tag containing a space yields a URL with a raw space (my harness observed …/download/desktop v1/…).
Why this is not a reachable defect: the release workflow constructs the tag as desktop-v$version only after validating version against ^[0-9]+\.[0-9]+\.[0-9]+([+-][0-9A-Za-z.-]+)?$ (desktop-release.yml, "Resolve version" step) and exiting 1 otherwise. That character set (digits, ., -, +, alphanumerics) is all URL-path-safe, and a space is rejected upstream — my workflow harness confirmed INPUT_VERSION='0.1.0 bad' exits 1. So the raw interpolation can never receive a character requiring encoding via the real pipeline. Reported for completeness as a robustness inconsistency (artifact encoded, tag not), not a merge condition.
Not covered
- Rust/Tauri native crate (1511 lines:
main.rs678,runtime.rs547,desktop_state.rs286) — not compiled or run. This container has nocargo/rustc. The crate is the heart of the feature (window/navigation policy, command origin gating, updater, daemon supervision). I read-reviewed it and found a sound design — every#[tauri::command]is gated behindrequire_bootstrap_origin(onlytauri://localhostmay invoke; the runtime Web Shell origin127.0.0.1:portcannot), navigation is locked to the bootstrap URL + the recorded loopback origin,origin_ofrejects non-loopback, new-window/download handlers are scheme- and origin-filtered, and the source carries unit tests for these predicates — but read-review is not a compile or a test run. The PR adds adesktop_shellCI job (ci.yml) that runscargo teston every PR touching the shell; that gate exists but cannot run here. - Packaged-app builds and smoke (
smoke:packaged,smoke:runtime,tauri build) — not run. Require Tauri, the bundled Node runtime (build:runtime), and (for the reference path) macOS.smoke-packaged.jswas read-reviewed (coherent: HTML shell served unauthenticated with noSet-Cookie,/capabilitiesstays 401) but not executed. - Production signing / notarization (macOS Developer ID, Windows Authenticode) and Windows/Linux installer launch — require repository secrets / native installers; the PR itself states these are release-CI-validated and not launched locally.
shellcheck,actionlint,yamllint— not run (binaries absent in-container). Substitutedbash -non all 84 bashrun:blocks (clean) and a js-yaml parse of both workflows (valid). The 3pwshblocks were not syntax-checked (no PowerShell available).- Per-commit attribution — the checkout is depth 2 (merge commit + base tip + PR head only); the 32 commits are not individually reachable, so this verifies the aggregate
HEAD^1..HEADdiff, not each commit. - Full end-to-end packaged launch — I verified the integration contract (fragment token, deep-health gating, bearer-only gate) against the real daemon/web-shell code, but did not observe a real bundled daemon boot inside a Tauri webview. This reproduces the shape of the auth handshake, not a packaged launch.
Methodology
Environment: the CI verify container (node:22-bookworm, Linux), working tree at refs/pull/8132/merge (depth 2); npm ci + npm run build already run at HEAD. No cargo/rustc, no Tauri, no shellcheck/actionlint/yamllint. PR metadata read from $QWEN_VERIFY_CONTEXT; no GitHub token, no network writes.
How each harness drove the code (all mock-free with respect to the unit under test, kept under harness/):
serve-auth-ab.sh— ran the realcreateServeAppvia vitest against the unmodified daemon (cell A) and against a scratch cookie-accepting mutation ofbearerAuth(cell B), restoringauth.tsvia trap; verdict from vitest exit codes and the assertion message.release-edge.mjs— drove the realcreate-desktop-update-manifest.mjsandversion.jsviachild_processacross 27 edge cases (ambiguous/missing artifacts, missing signature, missing args, URL-encoding, invalid/v-prefixed/prerelease/idempotent versions).workflow-check.mjs— parsedci.yml+desktop-release.ymlwith js-yaml, extracted everyrun:block verbatim,bash -n'd the 84 bash blocks, and executed the fail-open change filter (stubbedgit) and the semver guard with controlled inputs.- Corroborating gates (ran, exit 0, not folded into the 46-counter to avoid double-counting): the PR's own
scripts/test-release.js; web-shellclient/config/daemon.test.ts(19/19); the new serve guard test on head (1/1).
Raw logs: logs/release-edge.json (27/0), logs/workflow-check.json (17/0; 84 bash blocks clean). Evidence captures: evidence/01-serve-auth-ab.png, 02-release-edge-cases.png, 03-workflow-bash-filter.png, 04-pr-test-release.png. The 46 assertions in assertions.json = 2 (A/B) + 27 (release-edge) + 17 (workflow); each maps to a scripted check that executed.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Thanks for the PR — this is a substantial piece of work, and it reads well. Template looks complete ✓ — every required section is present, with a full bilingual body. Problem. This is a Direction. The approach is aligned with #8092, and the code reflects it — the shell is deliberately thin around Size. No core production paths are touched — this adds a new, isolated Approach. The scope feels coherent for "release-ready desktop app," and the changes to existing files are all directly necessary (workspace isolation in Risk. No elevated signal from the revert-history lens — none of the changed files match the high-risk paths (no Moving on to code review. 🔍 中文说明感谢这个 PR —— 工作量很大,而且写得很扎实。 模板 完整 ✓ —— 所有必需小节齐全,且有完整的双语正文。 问题。 这是一个 方向。 方案与 #8092 一致,代码也体现了这一点 —— shell 刻意保持轻薄,包裹 规模。 未触及核心生产路径 —— 本 PR 新增了一个隔离的 方案。 对于"可发布的桌面应用"这个目标,范围是连贯的;对现有文件的改动都是直接必需的( 风险。 从 revert 历史视角看无升级信号 —— 改动文件均未命中高风险路径(无 进入代码审查。🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewMy independent take before reading the diff: a thin Tauri shell that spawns the existing The PR matches that almost exactly and goes further, and the implementation is genuinely security-conscious. A few things done well:
No critical blockers found, and no clear AGENTS.md violations — the package is correctly isolated (own npm + Cargo lockfiles, excluded from the root workspace), eslint is scoped, and the code is Rust + browser JS + scripts rather than shared TS. Two non-blocking questions for the maintainer:
The startup + auth flow the maintainer is signing off on: sequenceDiagram
participant P1 as User
participant P2 as Bootstrap UI
participant P3 as Tauri shell
participant P4 as Daemon qwen serve
participant P5 as Web Shell
P1->>P3: choose workspace
P3->>P4: spawn on 127.0.0.1 with random token
P4-->>P3: report listening URL
P3->>P4: deep health check with Bearer token
P4-->>P3: status ok
P3->>P2: show recovery page until healthy
P3->>P5: navigate WebView to daemon origin, token in fragment
P5->>P4: exchange fragment token, then Bearer auth
P5-->>P1: Web Shell ready
Files changed (12 of 87 shown — the rest are icons/binary)
Test evidence (the PR's own CI)This is an unattended CI run, so I did not build or run any PR code — the signal below is the PR's own CI on the reviewed commit, read via the API. CI is fully green and settled (no failures, no pending
The notable one is Desktop Shell (ubuntu-22.04) — the job this PR adds — which compiles the Tauri crate ( Sandboxed verification would settle the behavioural claim: Real-scenario tmux testing is not run on the CI path (it's local-invocation only); the live-behaviour signal here comes from the lane above. 中文说明代码审查我在看 diff 之前的独立方案:一个轻薄的 Tauri shell,在 loopback 上以随机 token 启动现有 PR 几乎完全符合这个方案并且做得更多,实现确实注重安全。几个做得好的点:
未发现关键阻塞项,也没有明显的 AGENTS.md 违规 —— 包正确隔离(独立 npm + Cargo lockfile,排除在根 workspace 外),eslint 有范围限定,代码是 Rust + 浏览器 JS + 脚本而非共享 TS。 给维护者的两个非阻塞问题:
测试证据(PR 自带 CI)这是无人值守的 CI 运行,所以我没有构建或运行任何 PR 代码 —— 下面的信号是 PR 自身在被审 commit 上的 CI,通过 API 读取。CI 全绿且已稳定(无失败、无 pending 的 沙箱验证可以坐实行为性结论: CI 路径上不跑真实场景 tmux 测试(仅限本地调用);此处的实时行为信号来自上面的验证通道。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review and green CI, but this is a large new product surface touching auth, release/signing, and auto-update, so it needs a maintainer's product-direction and security sign-off rather than an automated approval. Stepping back: this is good work. The implementation matches and exceeds my independent proposal — it's thin around It isn't small, though. It's ~4266 production lines introducing a whole new desktop product surface from a fork, and it touches exactly the boundaries the gate escalates: the auth model (fragment-token bootstrap, Bearer-only daemon auth, cookies deliberately removed), the release and signing pipeline (Developer ID + notarization, Authenticode, updater signing), and auto-update. None of these are defects I can pin as blockers — the code reads as correct and well-tested. They're "a human maintainer must own this decision." There's also one thing I'd want confirmed before merge: the Web Shell client side of the fragment→Bearer exchange isn't in this diff, so a maintainer should verify that piece is already in place rather than assumed. So I'm not approving and not requesting changes — I'm deferring, because the right call here is a maintainer's, not an automated gate's. ⏸️ Deferring to @doudouOUC — this PR is high quality and CI is green, but it introduces a new desktop product surface and touches the auth boundary and the release/signing/update pipeline, which need a maintainer's product-direction and security sign-off. Specifically worth a human look: (1) confirm the Web Shell client side of the fragment-token → Bearer exchange is already in place (no 中文说明置信度:3/5 —— 审查干净、CI 全绿,但这是一个触及认证、发布/签名与自动更新的大型新产品表面,需要维护者做产品方向与安全把关,而非自动批准。 退一步看:这是高质量的工作。实现符合并超过了我的独立方案 —— 对 但它不小。这是约 4266 行生产代码、从 fork 引入的一整个新桌面产品表面,并且触及了 gate 会升级的边界:认证模型(fragment-token bootstrap、仅 Bearer 的 daemon 认证、刻意移除 cookie)、发布与签名流水线(Developer ID + 公证、Authenticode、updater 签名)以及自动更新。这些都不是我能 pinpoint 为阻塞项的缺陷 —— 代码读起来正确且测试充分。它们属于"应由人类维护者拍板的决定"。还有一点我希望在合并前确认:fragment→Bearer 交换的 Web Shell 客户端侧不在本 diff 中,所以维护者应确认该部分已就位,而不是被假设存在。 所以我不批准、也不要求修改 —— 我转交维护者,因为这里正确的决定应由维护者做出,而非自动 gate。 ⏸️ 转交 @doudouOUC —— 本 PR 质量高、CI 全绿,但它引入了一个新的桌面产品表面,并触及认证边界与发布/签名/更新流水线,需要维护者做产品方向与安全把关。具体值得人工查看:(1) 确认 fragment-token → Bearer 交换的 Web Shell 客户端侧已就位(diff 中没有 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Maintainer verification — built and ran this PR on a real macOS machineI built the full desktop shell from this PR's head ( Environment: macOS 26.6 (Apple Silicon, arm64) · Node 22.23.2 · Rust 1.97.1 stable · 1. The documented test plan
The bundled runtime manifest correctly pins provenance: { "desktopVersion": "0.0.1", "qwenCodeVersion": "0.21.3",
"qwenCodeCommit": "cb546668a43d75c86737ae555398af5b53b95182",
"target": "darwin-arm64", "node": "v22.23.2" }The built 2. Live UX flows in the installed appLaunched First run — no saved workspace. Shows the picker; no Retry button (correct for the idle state). The window also restored to exactly the bounds I pre-seeded in Runtime ready — the real Web Shell in the native window, serving the selected workspace ( Daemon crash recovery. I Retry. Clicking Retry started a fresh daemon with a new token and recovered into the Web Shell in the same window. Also verified live:
3. Independent security probes against the running appI did not rely on the PR's own smoke script for these — separate probe against the live daemon: 11/11 passed. The fragment-token design holds in practice: 4. Release pipeline, exercised against real artifactsRather than trust the workflow by reading it, I replayed its steps locally on the actual build output. Artifact naming — ran the These match Updater signature is genuinely valid, verified independently of Tauri: Cross-platform runtime prep works from this Mac — CI job is real, not a no-op. The new 5. Guards actually fail when they should (mutation tests)Green checks only mean something if they can go red. I broke each guard on purpose:
6. Non-blocking notesa) The PR description is stale about cookies. It says the app "exchanges the one-shot URL token for an HttpOnly b) Long workspace paths overflow the bootstrap card (visible in the crash-recovery screenshot above — the path row runs past the panel's rounded edge). /* bootstrap/index.html — on the wrapper <div> at line 247 */
min-width: 0;
/* or on .status */
grid-template-columns: 22px minmax(0, 1fr);c) Recovery copy says "could not start" for a daemon that died after starting. The design doc distinguishes d) A hard Two smaller nits: VerdictThe packaging, lifecycle, security boundary, integrity, and release contracts all hold up under real execution on macOS arm64, and the guards fail correctly when broken. LGTM from my side for merge, with (a) worth fixing in the description and (b) a nice one-line follow-up. Not covered by my run: Windows/Linux native installers were not built or launched locally (no runners for them here) — I only verified their bundled runtimes prepare correctly cross-target. Apple notarization and Windows Authenticode need repository secrets and remain release-CI-only, as the PR states. 中文版本维护者验证 —— 在真实 macOS 机器上构建并运行我在真机上从本 PR 的 head( 环境: macOS 26.6(Apple Silicon,arm64)· Node 22.23.2 · Rust 1.97.1 stable · 1. 文档中的测试计划
bundled runtime 的 manifest 正确记录了来源: 构建出的 2. 已安装应用的真实 UX 流程在隔离
还验证了:
3. 独立的安全探测这些没有依赖 PR 自带的 smoke 脚本,而是对运行中的 daemon 单独探测:11/11 通过。 未带 token 的 fragment token 的设计在实践中成立:daemon 的 45 行请求日志里 4. 用真实产物验证发布链路我没有只靠读 workflow,而是在真实构建产物上本地复刻了它的步骤。 产物命名 —— 原样执行 与 updater 签名确实有效(脱离 Tauri 独立验证):minisign 跨平台 runtime 准备在这台 Mac 上可用: CI job 是真跑的,不是空转:本 PR 上新增的 5. 护栏确实会在该红的时候变红(变异测试)只有能变红的绿灯才有意义。我逐个人为破坏了这些护栏:
6. 非阻塞说明a) PR 描述关于 cookie 的部分已过时。 描述里写"用一次性 URL token 换取 HttpOnly b) 过长的工作区路径会撑破 bootstrap 卡片(见上面崩溃恢复截图,路径行越过了面板圆角边界)。 /* bootstrap/index.html —— 给第 247 行的包裹 <div> 加 */
min-width: 0;
/* 或者改 .status */
grid-template-columns: 22px minmax(0, 1fr);c) 启动成功后才崩溃的 daemon,恢复页仍显示"could not start"。 设计文档区分了 d) 对桌面壳发 另有两个小点: 结论打包、生命周期、安全边界、完整性和发布契约在 macOS arm64 上真实运行时全部成立,护栏被破坏时也会正确失败。我这边 LGTM,可以合并;建议顺手修掉 (a) 的描述,(b) 可作为一行的后续跟进。 本次未覆盖: Windows / Linux 原生安装包没有在本地构建或启动(这里没有对应 runner),我只验证了它们的 bundled runtime 能正确跨平台准备。Apple 公证和 Windows Authenticode 依赖仓库 secrets,仍然只能由 release CI 验证,与 PR 中的说明一致。 |
Review Summary — PR #8132Reviewed: 80+ files, +9469 at What this PR doesTurns the Tauri PoC into a release-ready desktop shell that packages the existing Web Shell. The native layer owns: workspace selection/persistence, daemon lifecycle (start/monitor/recover), authenticated loopback bootstrap (fragment-based token → HttpOnly cookie exchange), safe navigation boundaries, window state persistence, silent update checks, and a multi-platform release pipeline (macOS arm64/x64, Windows x64, Linux x64). Architecture & Security — Strengths
Observations[Low] [Low] [Info] [Info] [Info] ci-bot's repeated CHANGES_REQUESTED is about the author's own beta-readiness checklist where productization items (packaged-app E2E, notarization, security review) are explicitly deferred to follow-ups under #8092. This is a scope decision, not a code defect. The PR description states this clearly. VerdictWell-architected desktop shell with a thoughtful security model. The Rust code is clean, error handling is thorough, and the release pipeline has proper safety gates. Two human reviewers (wenshao, DragonnZhang) have already approved. CI is green. Approve. |
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. Strong security model: navigation lockdown, bootstrap origin gating, loopback-only runtime, fragment-based auth, strict CSP, minimal capabilities. Rust code is clean with generation-based race protection and poisoned mutex recovery. Release pipeline has proper safety gates (dry_run default, SemVer validation, main-branch enforcement). The ci-bot's CHANGES_REQUESTED is about deferred productization scope, not code defects.
…enLM#8372) The desktop-shell filter used `git diff --name-only FETCH_HEAD HEAD` against a depth-1 fetch of the base. That is a two-tree diff, not the PR's diff: it reports every file the base gained since the branch point as a change on the PR's side. QwenLM#8132 added packages/desktop-shell, so from then on every branch older than it saw 78 phantom desktop-shell paths, decided the crate had changed, and ran the job -- inside a checkout of refs/pull/N/head, which does not contain the directory. The step then died on a working directory that was never there: Error: The cwd: .../packages/desktop-shell/src-tauri does not exist! Measured on such a branch: 78 desktop-shell paths from `git diff BASE HEAD`, 0 from `git diff BASE...HEAD`, and the job's own log reporting `desktop-shell changed: true` for a PR that touched two files, neither of them the crate. Ask the API instead -- the same call the CI profile classifier a few hundred lines above already makes, so the file now answers this question one way. `previous_filename` is included because renaming a file out of the crate changes it and only the old path says so. Fail-open then needed a floor of its own: "any uncertainty runs the job" is right until the tree has no crate to compile, where it means a red check rather than a cautious one. A head without src-tauri/Cargo.toml cannot have regressed the crate, so the filter reports no change and says why. Driven through the six cases with the step lifted out and `gh` stubbed -- docs-only/touched/absent-crate/API-failure both ways/rename-out -- and the new query run against this PR on the real API, which returns its two files and matches nothing.
|
Released in v0.21.4. |













What this PR does
This PR turns the isolated Tauri proof of concept into a release-ready desktop shell that packages the existing Web Shell instead of maintaining another desktop UI.
The app now owns the native lifecycle around the shared Web Shell: visible startup and recovery states, workspace selection and persistence, single-instance window behavior, daemon crash recovery, diagnostics, authenticated loopback bootstrap, safe navigation, and automatic update checks.
The release pipeline builds self-contained macOS, Windows, and Linux packages with a platform-specific Node.js runtime and the current Qwen Code/Web Shell bundle. Published builds require updater signing on every platform, Developer ID signing and notarization on macOS, and Authenticode signing on Windows. The workflow also emits checksums, release assets, and the stable updater manifest.
The package stays outside the root npm workspace and keeps its own npm and Cargo lockfiles, so Tauri remains isolated from the main Qwen Code installation.
Why it's needed
The existing desktop implementation and Web Shell are separate UI and product surfaces. That duplicates feature work and makes desktop behavior more likely to fall behind.
This follows the lower-maintenance direction from #8092: Web Shell remains the shared product surface, while the native layer is limited to packaging, lifecycle, security boundaries, updates, and platform integration.
Reviewer Test Plan
How to verify
packages/desktop-shell, runnpm ci,npm run build:runtime,npm test,npm run test:release, andnpm run smoke:runtime.CI=false TAURI_SIGNING_PRIVATE_KEY=<test-key> TAURI_SIGNING_PRIVATE_KEY_PASSWORD='' npm run tauri -- build --bundles app --config '{"bundle":{"macOS":{"signingIdentity":"-"}}}'.npm run smoke:packaged -- src-tauri/target/release/bundle/macos/Qwen\ Code.app/Contents/MacOS/qwen-code-desktopand confirm the bundled daemon, token bootstrap, HttpOnly cookie, and authenticated Web Shell API path succeed.Desktop Releasewithdry_run=true; all four target jobs should build, smoke test, and upload their installers without publishing a GitHub release.Evidence (Before & After)
Before: the PoC could launch the Web Shell on macOS, but it did not have production startup/recovery UX, persistent desktop state, secure installed-app bootstrap, release signing, automatic updates, or a complete multi-platform release path.
After: the installed macOS app launches the bundled authenticated runtime, exchanges the one-shot URL token for an HttpOnly
SameSite=Strictcookie, redirects to a token-free URL, initializes Web Shell APIs successfully, and reaps the daemon on exit. The same release contract is encoded for macOS arm64/x64, Windows x64, and Linux x64.Tested on
Environment (optional)
macOS arm64, Node.js 22, Tauri 2. Local verification covered the packaged
.app, installed-app authentication, Rust tests, release-helper tests, Clippy, repository lint and typecheck, focused daemon browser-auth tests, authenticated runtime smoke, updater signing, and daemon cleanup.Risk & Scope
Linked Issues
Refs #8092
中文说明
这个 PR 做了什么
这个 PR 将隔离的 Tauri 概念验证完善为可发布的桌面壳,直接打包现有 Web Shell,不再维护另一套桌面 UI。
应用现在负责共享 Web Shell 外围的原生生命周期:可见的启动和恢复状态、工作区选择与持久化、单实例窗口行为、daemon 崩溃恢复、诊断能力、带认证的 loopback bootstrap、安全导航和自动更新检查。
发布流水线会为 macOS、Windows 和 Linux 构建自包含安装包,其中包括平台 Node.js runtime、当前 Qwen Code bundle 和 Web Shell。正式发布要求所有平台提供 updater 签名,macOS 使用 Developer ID 签名和公证,Windows 使用 Authenticode 签名。流水线还会生成校验和、Release 产物和稳定更新清单。
该包仍位于根 npm workspace 之外,并维护独立的 npm 和 Cargo lockfile,因此 Tauri 依赖不会进入 Qwen Code 主安装链路。
为什么需要
现有桌面端与 Web Shell 是两套独立的 UI 和产品表面,会重复消耗功能开发成本,也更容易让桌面端能力落后。
这个方案沿用 #8092 中的低维护方向:Web Shell 保持为共享产品表面,原生层只负责打包、生命周期、安全边界、更新和平台集成。
Reviewer 测试计划
如何验证
packages/desktop-shell下运行npm ci、npm run build:runtime、npm test、npm run test:release和npm run smoke:runtime。npm run smoke:packaged,确认 bundled daemon、一次性 token bootstrap、HttpOnly cookie 和带认证的 Web Shell API 链路成功。dry_run=true触发Desktop Release,确认四个目标都能构建、smoke test 并上传安装包,但不会发布 GitHub Release。前后对比证据
之前:PoC 可以在 macOS 启动 Web Shell,但没有产品化启动/恢复体验、持久化桌面状态、安全的已安装应用认证、发布签名、自动更新或完整的多平台发布链路。
之后:安装后的 macOS 应用会启动 bundled authenticated runtime,用一次性 URL token 换取 HttpOnly、
SameSite=Strictcookie,重定向到无 token URL,成功初始化 Web Shell API,并在退出时回收 daemon。相同的发布契约已覆盖 macOS arm64/x64、Windows x64 和 Linux x64。已测试平台
环境
macOS arm64、Node.js 22、Tauri 2。本地验证覆盖 packaged
.app、已安装应用认证、Rust 测试、release helper 测试、Clippy、仓库 lint 和 typecheck、daemon browser-auth 定向测试、authenticated runtime smoke、updater 签名和 daemon 清理。风险与范围
关联 Issue
Refs #8092