fix(desktop): strip Windows verbatim prefix from runtime spawn paths - #8936
Conversation
Tauri's resource_dir() returns \\?\ verbatim paths on Windows, so the bundled Node executable and the cli-entry.js argument were spawned with that prefix. Node's entry-script resolution cannot handle it and dies with EISDIR before the daemon reports its listening URL, making the app unstartable regardless of workspace. Simplify both paths with dunce before spawning, complementing the workspace-path fix from #8615. Fixes #8929
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. #8929 is a P1 startup crash — Windows Desktop 0.1.0 cannot start at all, dying with Direction: aligned — a desktop app that doesn't launch on Windows is about as core as it gets. No related claude-code CHANGELOG entry, but this is a launcher bug specific to our Tauri desktop shell. Size: not applicable — Approach: minimal, and the fix the issue itself suggests. Path construction for node/entry is extracted into Risk: no elevated risk signals (no revert-correlated path matches). One edge the PR itself calls out: when stripping would be unsafe (>260 chars, trailing dot/space) Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的真实 bug，而非理论问题。#8929 是 P1 启动崩溃——Windows 桌面端 0.1.0 完全无法启动，在守护进程打印监听 URL 之前就以 方向：对齐——桌面端在 Windows 上无法启动是最核心的问题。claude-code CHANGELOG 无相关条目，但这是我们 Tauri 桌面外壳特有的启动器 bug。 规模：不适用—— 方案：最小改动，正是 issue 建议的修法。node/entry 路径构造提取为 风险：无升级风险信号（未命中 revert 相关路径）。PR 已自述一个边缘情况：当剥离不安全时（超 260 字符、结尾点/空格）， 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review Independent take before reading the diff: for "Tauri hands us Reading the diff against the base code, nothing blocking:
No correctness, security, or convention issues found. Testing Unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit. The load-bearing job is green: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 What CI cannot settle: end-to-end startup of a packaged Windows build — the author flags this too. Neither sandbox lane applies here: 中文说明代码审查 读 diff 前先给出独立方案：针对 "Tauri 给出 对照基础代码审查 diff，无阻塞问题：
未发现正确性、安全性或规范问题。 测试 无人值守 CI 运行——此处未构建或执行任何 PR 代码；以下证据来自 PR 自身在受审 commit 上的 CI。关键任务已绿： （上方 CI 表格在 CI 结束后由 finalize 流程原地更新。） CI 无法定论的一点：打包后 Windows 构建的端到端启动——作者也已注明。两个沙箱通道在此均不适用： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — a fully-evidenced P1 startup-crash fix, and the change is exactly the minimal mechanism the evidence points to. Stepping back: the problem is as well-evidenced as a gate could ask — a captured spawn command line, a standalone reproduction of the Node failure, and a workaround that proves the causal chain. The PR mechanizes that workaround for the two remaining spawn arguments; the approach matches the one I proposed independently before reading the diff; and nothing in the diff goes beyond what the goal needs. The test pins the fix's actual mechanism and passed on a real Windows runner. The one unverified piece — packaged Windows build startup — is inherently a release-time smoke test (no bot lane can execute it), and the PR says so itself. Six months from now, maintaining this: a small, commented helper with a spelled-out edge-case tradeoff — thank them, not curse them. CI is not fully settled yet ( 中文说明置信度：5/5 —— 证据完备的 P1 启动崩溃修复，改动恰好是证据指向的最小机制。 退一步看：这个问题的证据是审查门槛所能要求的最完备级别——抓到的 spawn 命令行、Node 失败的独立复现、以及证明因果链的 workaround。PR 把这个 workaround 机制化到剩余的两个 spawn 参数上；方案与我在读 diff 之前独立提出的方案一致；diff 中没有任何超出目标所需的改动。测试直接钉住修复机制本身，且已在真实 Windows runner 上通过。唯一未验证的部分——打包后 Windows 构建的启动——本质上只能靠发布前冒烟测试（任何机器人通道都无法执行），PR 也已自述。半年后维护这段代码：一个小巧、注释到位、边缘情况取舍明确的 helper——值得感谢而非吐槽。 CI 尚未全部结束（ — Qwen Code · qwen3.8-max Reviewed at |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed exact head cc74ccfcc80e5467e2919e310c8dc911ae8bb56e. The runtime-path change looks correct: every RuntimeLayout root flows through dunce::simplified, clippy is clean, and both Desktop Shell CI jobs pass. I did not find a blocking code issue.
Non-blocking linkage follow-up: #8929 also records the separate visible Windows runtime console bug (CREATE_NO_WINDOW), but no separate issue currently tracks it. Since this PR uses Fixes #8929, split that follow-up into its own issue before merge so it is not silently closed.
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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): This PR (QwenLM/qwen-code #8936) fixes issue #8929: on Wi...: none — all checks above completed within budget.; This PR (QwenLM/qwen-code #8936) fixes issue #8929: on Wi...: none — all planned checks completed within budget..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：This PR (QwenLM/qwen-code #8936) fixes issue #8929: on Wi...：none — all checks above completed within budget.；This PR (QwenLM/qwen-code #8936) fixes issue #8929: on Wi...：none — all planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Tauri's resource_dir() returns `\\?\` verbatim paths on Windows, and | ||
| // Node's entry-script resolution cannot handle that prefix (#8929). |
There was a problem hiding this comment.
[Suggestion] Fixes #8929 auto-closes the issue's second, still-unfixed bug — the visible runtime console window on Windows — which this very fix un-masks for every Windows user. The issue body's "Follow-up: visible runtime console window" section reports that the bundled runtime (qwen serve) appears in its own console window and closing that console kills the whole app; the issue's triage confirmed this is unfixed on main (CREATE_NO_WINDOW is set nowhere in this crate) and suggested tracking it separately or folding it into the same fix. This PR does neither, and no separate issue exists. — Failure scenario: after merge, GitHub closes #8929 — the only record of the console-window bug — while the fixed startup path makes the window newly visible on every Windows startup (previously masked by the instant crash). Users see a blank console window beside the app, and closing it terminates the runtime and the whole app, with no issue left tracking the behavior.
Suggested fix: either add the CREATE_NO_WINDOW creation flag to the runtime spawn (cfg-gated std::os::windows::process::CommandExt::creation_flags on the Command before group_spawn()), or file a follow-up issue for the console window and reference it in the PR description before merging, so closing #8929 does not silently drop that half of the report.
中文说明
Fixes #8929 会自动关闭该 issue 中第二个尚未修复的 bug —— Windows 上可见的运行时控制台窗口 —— 而本修复恰好会让这个窗口对每个 Windows 用户显现出来。issue 正文的 "Follow-up: visible runtime console window" 部分指出：内置运行时（qwen serve）会出现在独立的控制台窗口中，关闭该窗口会导致整个应用退出；issue 的 triage 已确认该问题在 main 上仍未修复（整个 crate 中没有设置任何 CREATE_NO_WINDOW），并建议单独跟踪或并入同一修复。本 PR 两者都没有做，且目前也没有单独的 issue 跟踪此问题。— 失败场景：合并后 GitHub 关闭 #8929（控制台窗口 bug 的唯一记录），而修复后的启动路径使该窗口在每次 Windows 启动时新出现（此前因运行时立即崩溃而被掩盖）。用户会在应用旁看到一个空白控制台窗口，关闭它会导致运行时和整个应用退出，且不再有 issue 跟踪该行为。
建议修复：为运行时 spawn 添加 CREATE_NO_WINDOW 创建标志（cfg 门控的 std::os::windows::process::CommandExt::creation_flags，在 group_spawn() 之前设置），或在合并前为控制台窗口问题创建一个后续 issue 并在 PR 描述中引用，避免关闭 #8929 时悄悄丢掉报告的另一半。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| assert!( | ||
| !path.starts_with("\\\\?\\"), | ||
| "runtime path keeps the verbatim prefix: {path}" | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The test asserts only the absence of the \\?\ prefix, never the resulting path values, so it cannot distinguish node from entry and passes for any prefix-free output — including value-corrupting mutations of the changed lines (swapping &node/&entry between the tuple slots, simplifying &root instead, returning an empty or mis-joined PathBuf). This is the only test covering layout_from_root, and the desktop_shell CI job runs without the bundled runtime, so such a regression would surface only at packaged-app runtime. — Failure scenario: a future refactor of layout_from_root that swaps the node/entry slots or mis-joins a segment stays green here (an empty string doesn't start with \\?\), and the wrong layout ships — spawning cli-entry.js as the executable fails desktop runtime startup on packaged Windows builds while this test passes.
Suggested fix: pin the full expected values instead of the loop (the assert_eq!s must run before node/entry are moved into the array):
assert_eq!(
node,
PathBuf::from(r"C:\Users\user\AppData\Local\Qwen Code Desktop\runtime\qwen-code\node\node.exe")
);
assert_eq!(
entry,
PathBuf::from(r"C:\Users\user\AppData\Local\Qwen Code Desktop\runtime\qwen-code\lib\cli-entry.js")
);中文说明
该测试只断言 \\?\ 前缀不存在，从不验证结果路径的具体值，因此无法区分 node 和 entry，任何不带前缀的输出都能通过 —— 包括对改动行的取值破坏型变异（在元组两个槽位间交换 &node/&entry、改为对 &root 做简化、返回空或拼接错误的 PathBuf）。这是覆盖 layout_from_root 的唯一测试，且 desktop_shell CI 任务不带内置运行时运行，因此此类回归只会在打包后的应用中暴露。— 失败场景：未来对 layout_from_root 的重构若交换了 node/entry 槽位或拼错路径段，此测试仍为绿色（空字符串同样不以 \\?\ 开头），错误布局会被放行 —— 把 cli-entry.js 当作可执行文件 spawn 会导致打包后的 Windows 构建无法启动桌面运行时，而此测试却显示通过。
建议修复：用完整期望值替代循环断言（assert_eq! 必须在 node/entry 被移入数组之前执行）：见上方英文部分的代码块。
— qwen3.8-max via Qwen Code /review (v0.21.9)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8936 fix(desktop): strip Windows verbatim prefix from runtime spawn paths
Type: Bug Fix (platform / Windows launcher) — scope/windows, priority/P1
Change size: +34/-6 across 1 file (packages/desktop-shell/src-tauri/src/runtime.rs)
Reviewed at HEAD: cc74ccfcc80e5467e2919e310c8dc911ae8bb56e (1 commit)
Findings Summary
- Critical: 0
- Major: 1
- Minor: 2
- Nit: 3
Key Observations
The mechanism is right and the PR is strictly better than the status quo: dunce::simplified is the correct tool (syntactic, no I/O, and require_file does the existence check immediately after), the extraction into layout_from_root puts all three root sources — QWEN_DESKTOP_RUNTIME_DIR, the debug_assertions path, and the packaged resource_dir() — behind one helper, and dunce was already a dependency so no lockfile churn.
The one substantive gap is a parallelism gap with #8619, the workspace half of this exact bug class, in this exact file. #8619 shipped a three-part pattern:
- strip with
dunce(dunce::canonicalize, runtime.rs:209), - reject any path where dunce refused to strip, with an actionable message (
ensure_supported_workspace_path, runtime.rs:194-205), - test both the strip and the rejection (3 tests, incl.
\\?\UNC\/\\?\GLOBALROOT\/ >260-char).
This PR ports (1) and half of (3), and drops (2) entirely. dunce::simplified returns the input unchanged and silently for any prefix that is not Prefix::VerbatimDisk — notably \\?\UNC\…, which is what fs::canonicalize yields for a mapped network drive or UNC install. In that case require_file still succeeds (Win32 accepts verbatim paths, so is_file() returns true) and the spawn then dies with exactly the EISDIR: lstat 'C:' / "Bundled runtime closed stdout before reporting its listening URL" that this PR exists to eliminate — with zero diagnostic pointing at the path. See M1.
Industry Context
dunce is the canonical Rust answer to the \\?\ problem and is already used by cargo/rustup/tauri for it, so the dependency and the simplified-over-canonicalize choice need no defence. The relevant industry pattern the PR misses is not the normalizer but the "normalize, then assert the invariant you actually need" discipline: a best-effort normalizer whose failure mode is "return the input unchanged" must be paired with a validation step, otherwise the caller silently inherits the un-normalized value. #8619 got this right in the same file; this PR relies on dunce alone.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
Fixes #8929 auto-closes the issue's still-open CREATE_NO_WINDOW console-window follow-up |
qwen-code-ci-bot (inline @180), yiliang114 (review body) |
Confirmed — independently found in Round 0 linked-issue verification. Verified creation_flags/CREATE_NO_WINDOW appear nowhere in src-tauri/src/**, and Command at runtime.rs:54-66 sets no creation flags. Agree it is non-blocking, but the follow-up must be split into its own issue before merge. Worth adding to Risk & Scope, which currently omits it. |
New test asserts only the absence of \\?\, never the resulting path values |
qwen-code-ci-bot (inline @613) |
Confirmed — same conclusion, reached independently. I add the missing other half below (no negative-case test at all — see m2). |
| Residual verbatim edge is "the right tradeoff"; "No correctness, security, or convention issues found" (confidence 5/5) | qwen-code-ci-bot (triage stage 2 & 3) |
Disagree. The tradeoff is right for the stripping decision — hard-stripping a >260-char or UNC path would indeed be wrong. But #8619 paired that same tradeoff with ensure_supported_workspace_path so the residual becomes a diagnosable error rather than a cryptic crash. That half is missing here, which is a real correctness/diagnosability gap, not a documented tradeoff. See M1. |
| "all three root sources … so both spawn args are simplified on every path" | qwen-code-ci-bot (triage stage 2) |
Partially inaccurate. Simplification is conditional and re-evaluated per path; node and entry have different suffix lengths and can therefore diverge. See m1. |
| M1 (Major): residual-verbatim guard from #8619 not ported to the runtime paths | — | New finding — runtime.rs:172-185 |
m1 (Minor): node/entry simplified independently → mixed plain/verbatim state reachable |
— | New finding — runtime.rs:181-184 |
| m2 (Minor): documented residual behavior has no negative-case test (#8619 shipped two) | — | New finding — runtime.rs:601-615 |
Nits: positional (PathBuf, PathBuf) return; root: PathBuf taken by value; assertion loop hides which path failed |
— (bot's @613 partially overlaps the third) | New / partial overlap |
Additional Audit Coverage
Independently checked, no issue found — recorded so the clean areas are explicit:
- Caller/consumer blast radius:
RuntimeLayouthas exactly one consumer (DesktopRuntime::start, runtime.rs:51-56);layout_from_rootis private with onlyresolveand the new test as callers. No downstream breakage, no public API surface added. - Convention compliance (AGENTS.md):
packages/desktop-shell/src-tauri/**is not one of the listed core-module paths, so the maintainer-only two-tier gate does not apply; the diff is minimal and adds no speculative abstraction, consistent with "Simplicity First". - Supply chain:
dunce = "1.0.5"already present insrc-tauri/Cargo.toml(added by #8619); noCargo.toml/Cargo.lockchange needed or made. Correct. - CI reachability of the new test:
desktop_shellmatrix is['ubuntu-22.04', 'windows-2022'](ci.yml:1133) and runscargo test --manifest-path src-tauri/Cargo.toml(ci.yml:1209), so the#[cfg(windows)]test does execute on a real Windows runner.fmt/clippyare deliberately not gated for this crate (ci.yml:1124-1125), so the new#[cfg(windows)] use super::layout_from_root;import placement carries no CI risk. - Security (adversarial pass): traced
dunce::is_safe_to_strip_unc— it rejects any non-VerbatimDiskprefix and any.., reserved (CON,COM4, …), or trailing-dot/space component. Stripping therefore cannot change the resolved target; no traversal or path-semantics change is introduced. #[cfg(windows)]gate on the new test is required, not merely conservative: off Windows the literal\\?\C:\…parses as a singleNormalcomponent andsimplifiedis a compile-time no-op, so the assertion would fail. Correctly gated — and correspondingly, this test contributes zero coverage on the Linux leg and on the author's macOS run.- Non-Windows cost: two extra
to_path_buf()allocations once per app start. Non-issue. - Error-message side effect:
require_filenow reports the simplified path, which is a small unadvertised improvement to the "runtime is missing at …" message. Benign.
Final Verdict
Needs Changes (non-blocking on the P1 itself). The fix is correct for the reported case and I would not hold the P1 behind polish — but M1 is ~8 lines that reuse a guard already living 10 lines below in this same file, and without it the exact failure mode this PR exists to eliminate remains silently reachable on UNC/mapped-drive and long-path installs. Recommend landing M1 + m2 with this PR, and splitting the CREATE_NO_WINDOW follow-up out of #8929 before merge as both other reviewers requested. m1 and the Nits are optional.
中文说明
结论：建议修改（不阻塞该 P1 修复本身）。
修复机制本身正确：dunce::simplified 是这里的正确工具（纯语法、无 I/O，且紧随其后的 require_file 负责存在性检查），把路径构造抽成 layout_from_root 让三个 root 来源（环境变量、debug、打包 resource_dir）统一走同一条路径，dunce 也已是现有依赖。
核心问题是与 #8619（同一文件、同一类 bug 的 workspace 那一半）不对等。#8619 落地的是三件套：① 用 dunce 剥离；② 对 dunce 拒绝剥离的残留路径给出可行动的报错（ensure_supported_workspace_path，runtime.rs:194-205）；③ 同时测试剥离成功与残留拒绝（3 个测试，含 \\?\UNC\、\\?\GLOBALROOT\、>260 字符）。本 PR 只落地了 ① 和 ③ 的一半，完全没有 ②。
dunce::simplified 对任何非 Prefix::VerbatimDisk 的前缀会静默原样返回 —— 典型就是映射网络驱动器 / UNC 安装时 fs::canonicalize 产出的 \\?\UNC\…。此时 require_file 依然通过（Win32 接受 verbatim 路径，is_file() 返回 true），随后 spawn 仍以 EISDIR: lstat 'C:' 崩溃 —— 正是本 PR 要消灭的症状，且没有任何指向路径的诊断信息。详见 M1。
交叉验证：已确认另两位评审的两条发现（Fixes #8929 会关掉未修的控制台窗口问题；新测试只断言前缀缺失）。不同意 triage 的「残留边界是正确取舍、无正确性问题、置信度 5/5」——取舍对的是「是否强行剥离」，缺的是 #8619 配套的残留校验。另外 triage 所述「两个 spawn 参数在所有路径上都被简化」并不精确：简化是按路径逐个条件判定的，node 与 entry 可能出现一个普通、一个 verbatim 的混合状态（m1）。
建议随本 PR 一并处理 M1 与 m2，并在合并前把 CREATE_NO_WINDOW 后续问题拆成独立 issue。
This review was generated by QoderWork AI
| dunce::simplified(&node).to_path_buf(), | ||
| dunce::simplified(&entry).to_path_buf(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
[Major] The residual-verbatim guard from #8619 is not ported to the runtime paths
dunce::simplified is best-effort: it strips only when is_safe_to_strip_unc passes, which requires the prefix to be Prefix::VerbatimDisk, every component to be a valid non-reserved Normal/RootDir, and the path to be within 260 chars. Anything else is returned unchanged and silently — and path.to_str() returning None (non-UTF-8 path) also falls back to the original.
That residual case has a precedent in this very file. #8619 — the workspace half of this same bug class — did not rely on dunce alone: it paired dunce::canonicalize with ensure_supported_workspace_path (runtime.rs:194-205) precisely so that a path dunce refuses to normalize surfaces as an actionable error instead of a crash. This PR ports the stripping but not the guard.
Concrete failure, not hypothetical — install or launch from a mapped network drive / UNC share:
resource_dir()canonicalizes, sorootbecomes\\?\UNC\server\share\….simplifiedcorrectly declines (notVerbatimDisk) and returns it verbatim.require_filesucceeds — the Win32 API accepts verbatim paths, sois_file()returnstrueand neither check fires.- The spawn then dies in
resolveMainPath→realpathSyncwith exactly theEISDIR: lstat 'C:'/ "Bundled runtime closed stdout before reporting its listening URL" of Windows: Qwen Code Desktop 0.1.0 fails to start - launcher passes \\?\ verbatim paths to bundled Node (EISDIR) #8929, with nothing in the message pointing at the path.
The same holds for a >260-char install path and for a non-UTF-8 path. Risk & Scope frames this as "behavior is unchanged from today rather than broken", which is true of the stripping decision — but the workspace half of this bug class converts that unchanged-and-broken state into a diagnosable one, and the runtime half now does not.
Suggested fix — generalize the helper that already exists 10 lines below and call it on both paths:
fn ensure_supported_path(path: &Path, description: &str) -> Result<(), String> {
if matches!(
path.components().next(),
Some(Component::Prefix(prefix)) if prefix.kind().is_verbatim()
) {
return Err(format!(
"{description} path uses an unsupported Windows extended-length form: {}. \
Choose a local drive path; network (UNC) shares, paths over 260 characters, \
and names ending in a dot or space are not supported.",
path.display()
));
}
Ok(())
}
// in RuntimeLayout::resolve, before the require_file calls:
ensure_supported_path(&node, "Node.js runtime")?;
ensure_supported_path(&entry, "Qwen Code runtime entry")?;Prefix::is_verbatim() already covers VerbatimDisk, VerbatimUNC and Verbatim, so this catches every residual form in one predicate. It also makes the per-path divergence flagged separately below unreachable.
中文说明
dunce::simplified 是 best-effort：只有当前缀为 Prefix::VerbatimDisk、所有组件合法且非保留名、路径不超过 260 字符时才剥离；其它情况（尤其 \\?\UNC\…）静默原样返回，非 UTF-8 路径也会回退到原值。
同一文件里已有先例：#8619 处理 workspace 时并没有只依赖 dunce，而是把 dunce::canonicalize 与 ensure_supported_workspace_path（runtime.rs:194-205）配对，让 dunce 拒绝规范化的路径以可行动的报错暴露出来，而不是崩溃。本 PR 只搬了剥离，没有搬校验。
真实失败场景（映射网络驱动器 / UNC 安装）：resource_dir() 规范化后 root 变成 \\?\UNC\server\share\… → simplified 正确地拒绝剥离 → require_file 依然通过（Win32 接受 verbatim 路径，is_file() 为 true）→ spawn 仍在 resolveMainPath → realpathSync 处以 EISDIR: lstat 'C:' 崩溃，报错信息里毫无路径线索。>260 字符与非 UTF-8 路径同理。
Risk & Scope 里「行为与现状一致，不会更糟」对「是否强行剥离」这个决定是成立的；但同类 bug 的 workspace 那一半把「一致地坏」变成了「可诊断」，runtime 这一半没有。
建议把下方已有的 helper 泛化后对两个路径都调用（见上方代码）。Prefix::is_verbatim() 已覆盖 VerbatimDisk/VerbatimUNC/Verbatim，一个判定即可兜住所有残留形式，同时也让下面单独提到的两路径分歧不可能发生。
This review was generated by QoderWork AI
| // Node's entry-script resolution cannot handle that prefix (#8929). | ||
| ( | ||
| dunce::simplified(&node).to_path_buf(), | ||
| dunce::simplified(&entry).to_path_buf(), |
There was a problem hiding this comment.
[Minor] node and entry are simplified independently, so a mixed plain/verbatim state is reachable
dunce::simplified re-evaluates is_safe_to_strip_unc for each argument, and one of its conditions is a length check (path_os_str.len() > 260 && windows_char_len(path_os_str) > 260). The two paths carry different suffixes — \node\node.exe (14 chars) vs \lib\cli-entry.js (17) — so there is a 3-character window of root lengths in which node is simplified to a plain path while entry keeps its \\?\ prefix.
That combination is the worst of both worlds: a plain program path spawned with a verbatim entry-script argument, i.e. the #8929 crash again, but from a layout that looks half-fixed in the logs. It also makes the triage note "both spawn args are simplified on every path" not quite exact — simplification is conditional and evaluated per path, not once per root.
Simplifying root once before the joins would keep the two consistent, at the cost of possibly emitting a plain >260-char path. Adding the residual guard suggested above is the cleaner fix, since it makes the mixed state impossible to reach silently rather than merely unlikely.
中文说明
dunce::simplified 会对每个入参各自重新判定 is_safe_to_strip_unc，其中包含长度检查（len() > 260 && windows_char_len() > 260）。两个路径后缀长度不同 —— \node\node.exe（14 字符）与 \lib\cli-entry.js（17 字符）—— 因此存在约 3 个字符的 root 长度窗口，使 node 被简化为普通路径、而 entry 仍保留 \\?\ 前缀。
这种组合是最糟的情况：用普通程序路径去 spawn 一个 verbatim 的入口脚本参数，即再次触发 #8929 崩溃，但日志上看起来像「已修一半」。这也说明 triage 所述「两个 spawn 参数在所有路径上都被简化」并不精确 —— 简化是按路径条件判定的，不是按 root 一次判定。
在 join 之前对 root 简化一次可以保证两者一致，代价是可能产出超过 260 字符的普通路径；更干净的做法是加上上面建议的残留校验，让混合状态无法被静默触达。
This review was generated by QoderWork AI
| "runtime path keeps the verbatim prefix: {path}" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
[Minor] The documented residual behavior has no test — #8619 shipped two for the workspace half
#8619 added three Windows-gated tests for this bug class: one positive (resolve_workspace_strips_windows_verbatim_prefix) and two negative (rejects_residual_windows_verbatim_workspace_paths pinning \\?\UNC\ and \\?\GLOBALROOT\, plus resolve_workspace_rejects_residual_verbatim_paths pinning the >260-char case). This PR adds only the positive one, so the exact edge case called out in Risk & Scope is unverified and free to change silently under a dunce version bump.
Secondly, because layout_from_root returns a positional (PathBuf, PathBuf) of two identically-typed values, for path in [node, entry] cannot distinguish them and the failure message names neither. Pinning the exact expected values — as the other reviewer also suggests — plus a case where the prefix is expected to survive would close both gaps:
assert_eq!(
node,
PathBuf::from(r"C:\Users\user\AppData\Local\Qwen Code Desktop\runtime\qwen-code\node\node.exe")
);
assert_eq!(
entry,
PathBuf::from(r"C:\Users\user\AppData\Local\Qwen Code Desktop\runtime\qwen-code\lib\cli-entry.js")
);Nits while here: layout_from_root could be RuntimeLayout::from_root(&Path) -> Self, which is type-safe against slot swaps and lets the test read layout.node/layout.entry by name; and root: PathBuf is taken by value although only &Path is needed (join does not consume).
中文说明
#8619 为同类 bug 加了三个 Windows 门控测试：一个正向（resolve_workspace_strips_windows_verbatim_prefix）和两个负向（rejects_residual_windows_verbatim_workspace_paths 钉住 \\?\UNC\ 与 \\?\GLOBALROOT\；resolve_workspace_rejects_residual_verbatim_paths 钉住 >260 字符）。本 PR 只加了正向那个，因此 Risk & Scope 里明确写出的边界情况没有任何测试保护，dunce 升级时行为可以静默变化。
其次，由于 layout_from_root 返回的是两个同类型值的位置元组 (PathBuf, PathBuf)，for path in [node, entry] 无法区分两者，断言失败信息也不会指明是哪一个。按另一位评审的建议钉住完整期望值，再补一个「前缀预期保留」的用例，即可同时补齐两处（见上方代码）。
顺带的 Nit：layout_from_root 可以写成 RuntimeLayout::from_root(&Path) -> Self，既能在类型上防止两个槽位被交换，也便于测试按 layout.node / layout.entry 具名读取；另外 root: PathBuf 是按值传入的，而实际只需要 &Path（join 不消耗所有权）。
This review was generated by QoderWork AI
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
LGTM. The change is minimal, correctly scoped, and well-tested.
What this PR does: Extracts the Node runtime and entry-script path construction into a layout_from_root helper, then applies dunce::simplified to both paths before returning them. This strips the \\?\ verbatim prefix that Tauri's resource_dir() returns on Windows, matching the same fix already applied to the workspace path in #8619.
Correctness: dunce::simplified is the right call — syntactic only, no filesystem access, keeps the verbatim form where stripping would be unsafe (>260 chars, trailing dot/space). The require_file check immediately after verifies the paths are valid regardless. Every RuntimeLayout root source flows through layout_from_root, so both spawn args are covered on all paths.
Testing: The Windows-gated regression test feeds a realistic \\?\C:\... root through the helper and asserts both outputs drop the prefix. Both Desktop Shell CI legs (Windows + Ubuntu) pass.
One non-blocking note: This PR uses Fixes #8929, but #8929 also tracks the separate CREATE_NO_WINDOW console bug. Consider splitting that follow-up into its own issue before merge so it is not silently closed.
|
Approved via |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. Clean, focused fix that correctly reuses dunce::simplified() consistent with the workspace-path fix in #8619. The Windows-gated test properly asserts both node and entry paths strip the \\?\ prefix.
Minor suggestions (non-blocking):
- Consider asserting the paths end with
node.exeandcli-entry.jsin the test, not just the prefix absence. - For the >260 char edge case where
dunce::simplifiedpreserves the verbatim form, a warning log would help future debugging.
|
Released in v0.21.10. |
5 participants
What this PR does
When the desktop shell spawns the bundled Node runtime on Windows, it now strips the
\\?\extended-length ("verbatim") prefix from the Node executable path and the CLI entry-script path before handing them to the spawn. The workspace path was already covered by #8619; this closes the same class of bug for the remaining two spawn arguments. A Windows-gated regression test asserts neither runtime spawn path keeps the prefix.Why it's needed
Fixes #8929. On Windows, Tauri's resource-directory resolution returns verbatim
\\?\paths, and the launcher passed them through to the spawn unchanged. Node's entry-script resolution (resolveMainPath→realpathSync) cannot handle that prefix and dies withEISDIR: lstat 'C:'before the daemon prints its listening URL — the app cannot start at all, regardless of which workspace is chosen. The reporter captured the actual spawn command via WMI and showed the prefix on the program path, the entry-script path, and the workspace value; the first two are what trigger the crash, so the workspace fix from #8619 alone is not sufficient. The reporter's workaround — pointing the runtime-directory environment variable at a plain path — confirms the runtime paths as the cause.Reviewer Test Plan
How to verify
cargo test --manifest-path src-tauri/Cargo.tomlinpackages/desktop-shellpasses (37 tests locally on macOS); the new Windows-gated test runs on thewindows-2022leg of thedesktop_shellCI job.\\?\for the Node executable or the entry script.Evidence (Before & After)
N/A — startup-crash fix in the Tauri launcher; no UI change. Before: "Bundled runtime closed stdout before reporting its listening URL" with an
EISDIRstack on every start attempt (see #8929). After: runtime starts as it does with the reporter's plain-path workaround.Tested on
Environment (optional)
cargo test/cargo clippy --testson the desktop crate; no bundled runtime needed.Risk & Scope
dunce::simplifiedkeeps the verbatim form when a simplified path would be unsafe (e.g. over 260 characters), so in that edge case behavior is unchanged from today rather than broken.Linked Issues
Fixes #8929. Related: #8615 / #8619 (workspace-path half of the same verbatim-path bug class).
中文说明
这个 PR 做了什么
桌面端外壳在 Windows 上拉起内置 Node 运行时之前，现在会先剥离 Node 可执行文件路径和 CLI 入口脚本路径上的
\\?\扩展长度（"verbatim"）前缀。workspace 路径此前已由 #8619 覆盖；本 PR 补齐了剩余两个 spawn 参数上的同类问题。并新增了一个仅在 Windows 上运行的回归测试，断言两个运行时 spawn 路径都不再保留该前缀。为什么需要
修复 #8929。在 Windows 上，Tauri 的资源目录解析返回 verbatim
\\?\路径，而启动器原样将其传给了 spawn。Node 的入口脚本解析（resolveMainPath→realpathSync）无法处理该前缀，会在守护进程打印监听 URL 之前以EISDIR: lstat 'C:'崩溃 —— 无论选择哪个 workspace，应用都完全无法启动。报告者通过 WMI 抓到了真实的 spawn 命令，显示程序路径、入口脚本路径和 workspace 值上都带有该前缀；其中前两个才是触发崩溃的原因，因此仅有 #8619 的 workspace 修复并不够。报告者的临时方案 —— 将运行时目录环境变量指向普通路径 —— 也印证了运行时路径正是问题根源。评审者测试计划
如何验证
packages/desktop-shell下运行cargo test --manifest-path src-tauri/Cargo.toml通过（本地 macOS 37 个测试全过）；新增的 Windows 门控测试会在desktop_shellCI 任务的windows-2022分支上运行。\\?\。证据（前后对比）
N/A —— Tauri 启动器中的启动崩溃修复，无 UI 变化。修复前：每次启动都报 "Bundled runtime closed stdout before reporting its listening URL" 并附
EISDIR堆栈（见 #8929）。修复后：运行时如报告者用普通路径 workaround 时一样正常启动。测试平台
环境（可选）
对桌面 crate 运行
cargo test/cargo clippy --tests；不需要内置运行时。风险与范围
dunce::simplified会保留 verbatim 形式，因此该边缘情况下行为与现状一致，而不会被改坏。关联 Issue
修复 #8929。相关：#8615 / #8619（同一 verbatim 路径问题中 workspace 路径的那一半）。