Skip to content

fix(acp-bridge): map Windows-shaped workspace paths to their sandbox mount - #7228

Merged
wenshao merged 9 commits into
QwenLM:mainfrom
zjunothing:fix/7139-serve-sandbox-cwd
Jul 21, 2026
Merged

fix(acp-bridge): map Windows-shaped workspace paths to their sandbox mount#7228
wenshao merged 9 commits into
QwenLM:mainfrom
zjunothing:fix/7139-serve-sandbox-cwd

Conversation

@zjunothing

Copy link
Copy Markdown
Collaborator

What this PR does

Makes canonicalizeWorkspace — the choke point every workspace path flows through (boot --workspace argv, client-registered workspaces, persisted registrations) — map Windows-shaped absolute paths to the sandbox launcher's mount convention (C:\work\proj/c/work/proj, mirroring getContainerPath in cli/src/utils/sandbox.ts) before resolution. The new translateWindowsWorkspaceForPosixSandbox helper is deliberately conservative: it only applies on POSIX inside a container sandbox (SANDBOX env set by the launcher; macOS sandbox-exec excluded, since seatbelt does not remap paths), only to <drive>:\… / <drive>:/…-shaped input, and only when the translated candidate actually exists on disk. Every other input — Windows hosts, unsandboxed runs, POSIX-shaped paths — passes through byte-for-byte unchanged.

Why it's needed

#7139 (P1): on Windows 11, qwen serve with QWEN_SANDBOX=docker relaunches itself into the Linux container correctly — docker inspect shows the right bind mount and WorkingDir, and direct docker exec works — yet every shell tool call fails with chdir(2) failed.: No such file or directory. The mechanism (confirmed against the triage's source read): the host-side launcher translates the mount and --workdir via getContainerPath, but entrypoint() forwards CLI arguments verbatim, so the in-container daemon receives --workspace C:\qwen-repro in host shape. On POSIX, path.resolve('C:\qwen-repro') treats the whole string as relative and prepends the cwd, producing a path that exists nowhere; the ACP child spawn's chdir then fails before any command runs. The same shape reaches the daemon through client-registered workspaces and persisted registrations (the runtime dir is mounted into the container), which is why the fix sits at the canonicalization choke point rather than at any single entry.

Reviewer Test Plan

How to verify

  1. Windows 11 + Docker Desktop (WSL2): $env:QWEN_SANDBOX="docker"; qwen serve --workspace "C:\qwen-repro" …, open a Web Shell session, run pwd / test -f marker.txt && echo FOUND_MARKER. Before this PR: every command fails with chdir(2) failed.: No such file or directory. After: commands run in /c/qwen-repro. (I don't have a Windows + Docker Desktop environment — validation on the reporter's setup is requested on the issue; see Risk & Scope.)
  2. Everywhere else, behavior is pinned unchanged: npx vitest run in packages/acp-bridge — 864/864, including the new guard matrix (Windows host / no sandbox / seatbelt / non-Windows-shaped input / translated mount missing → all inert) and a wiring regression test that fails on the unpatched source.
  3. macOS smoke (run locally): qwen serve --workspace <tmp> boots, GET /capabilities responds, POST /session creates a session with the correctly canonicalized workspaceCwd — the normal POSIX path through the changed function is untouched.

Evidence (Before & After)

The wiring regression test (workspacePaths.sandbox.test.ts) drives the real canonicalizeWorkspace with a stubbed SANDBOX env and a mocked mount-existence probe:

input (in-container) before fix after fix
C:\qwen-repro with SANDBOX set <cwd>/C:\qwen-repro (mangled → chdir ENOENT) ❌ /c/qwen-repro (the bind-mount location) ✅
C:\qwen-repro, no sandbox unchanged legacy behavior unchanged ✅
translated mount does not exist input returned untouched (never invent a path) ✅

Verified via git stash: the sandbox wiring test fails on the unpatched source. Local macOS qwen serve smoke: boot + POST /session{"sessionId": "…", "workspaceCwd": "/private/tmp/qwen7139-ws.…"} (canonicalization intact).

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS (Darwin 24.6), Node v22.23.1; vitest unit + wiring tests, real qwen serve HTTP smoke against packages/cli/dist. The Windows + Docker Desktop reproduction requires the reporter's environment.

Risk & Scope

  • Main risk or tradeoff: a translation applied too eagerly could redirect a legitimate path. The three-condition guard (POSIX container sandbox only, Windows-absolute shape only, translated target must exist) bounds this to exactly the launcher's own mount convention; a Linux directory literally named C:\x inside a sandbox whose /c/x also exists would be redirected — a deliberate trade against the P1.
  • Not validated / out of scope: the end-to-end Windows 11 + Docker Desktop flow is not reproducible on my hardware — the fix is derived from the triage's confirmed source read plus the launcher's own translation convention, and reporter validation is requested on the issue (same disclosure discipline as fix(mcp): use a dedicated undici fetch for Streamable HTTP transports #7195). The host-side launcher (entrypoint() forwarding args verbatim) is intentionally untouched: translating argv there would fix only the --workspace entry, while this choke point also covers client-registered and persisted workspace paths.
  • Breaking changes / migration notes: none — translateWindowsWorkspaceForPosixSandbox is a new export; canonicalizeWorkspace's behavior outside a container sandbox is byte-for-byte unchanged.

Linked Issues

Fixes #7139

中文说明

本 PR 做了什么

在所有 workspace 路径的收口点 canonicalizeWorkspace(启动 --workspace 参数、客户端注册、持久化注册都经过它)把 Windows 形状的绝对路径按 sandbox 启动器的挂载约定映射(C:\work\proj/c/work/proj,与 cli/src/utils/sandbox.tsgetContainerPath 一致)后再解析。新增的 translateWindowsWorkspaceForPosixSandbox 刻意保守:仅在 POSIX 容器沙箱内(启动器设置的 SANDBOX env;不重映射路径的 macOS sandbox-exec 排除)、仅对 <盘符>:\… 形状输入、且翻译后的目标真实存在时才生效。其余输入——Windows 宿主、非沙箱运行、POSIX 形状路径——逐字节原样通过。

为什么需要

#7139(P1):Windows 11 上 QWEN_SANDBOX=dockerqwen serve 能正确把自己重启进 Linux 容器——docker inspect 显示挂载与 WorkingDir 都正确、直接 docker exec 也正常——但每个 shell 工具调用都报 chdir(2) failed.: No such file or directory。机制(与 triage 源码分析相互印证):宿主侧启动器翻译了挂载与 --workdir,但 entrypoint() 把 CLI 参数原样转发,容器内 daemon 收到宿主形状的 --workspace C:\qwen-repro;POSIX 上 path.resolve 把整串当相对路径拼上 cwd,得到一个不存在的路径,ACP 子进程 spawn 的 chdir 在执行任何命令前就失败。同样形状还会经客户端注册与持久化注册(runtime 目录挂载进容器)到达 daemon——因此修复放在规范化收口点而非任何单一入口。

审阅测试计划

如何验证

  1. Windows 11 + Docker Desktop:按 issue 复现步骤,本 PR 之前每个命令 chdir(2) ENOENT;之后命令在 /c/qwen-repro 内正常执行(本机无 Windows+Docker 环境,已在 issue 请报告者验证,见风险与范围);
  2. 其他环境行为钉死不变:packages/acp-bridge 864/864,含守卫矩阵(Windows 宿主/无沙箱/seatbelt/非 Windows 形状/翻译目标不存在 → 全部不生效)与在未修复源码上失败的接线回归测试;
  3. macOS 冒烟(本地已跑):qwen serve 启动、GET /capabilities 正常、POST /session 返回正确规范化的 workspaceCwd——常规 POSIX 路径不受影响。

证据(Before & After)

接线回归测试用 stub 的 SANDBOX env + mock 的挂载存在性探针驱动真实 canonicalizeWorkspace:修复前 C:\qwen-repro 被拼成 <cwd>/C:\qwen-repro(即 chdir ENOENT 的来源);修复后解析为挂载位置 /c/qwen-repro;无沙箱时行为不变;翻译目标不存在时原样返回。git stash 验证该测试在未修复源码上失败。macOS 冒烟:boot + POST /session 返回 workspaceCwd 正常。

测试平台

macOS 已本地验证(✅);Windows / Linux 依赖 CI 与报告者真机(⚠️)。

环境

macOS(Darwin 24.6)、Node v22.23.1;vitest 单测/接线测试 + 真实 qwen serve HTTP 冒烟。Windows + Docker Desktop 复现依赖报告者环境。

风险与范围

  • 主要风险/权衡:翻译过度激进可能改写合法路径。三重守卫(仅 POSIX 容器沙箱、仅 Windows 绝对形状、翻译目标必须存在)把影响面限定在启动器自身的挂载约定内;沙箱内字面命名为 C:\x 且恰好 /c/x 存在的 Linux 目录会被重定向——这是对 P1 的有意取舍。
  • 未验证/超出范围:Windows 11 + Docker Desktop 全链路无法在本机复现——修复基于 triage 已确认的源码分析与启动器自身的翻译约定,已在 issue 请报告者验证(与 fix(mcp): use a dedicated undici fetch for Streamable HTTP transports #7195 相同的披露纪律)。宿主侧启动器(entrypoint() 原样转发参数)有意不动:在那里翻译 argv 只能覆盖 --workspace 一个入口,而本收口点同时覆盖客户端注册与持久化注册路径。
  • 破坏性变更/迁移说明:无——translateWindowsWorkspaceForPosixSandbox 为新导出;非容器沙箱下 canonicalizeWorkspace 行为逐字节不变。

关联 Issue

Fixes #7139

🤖 Generated with Claude Code

…mount

A Windows host relaunching `qwen serve` into a Linux Docker/Podman
sandbox translates the bind mount and --workdir via getContainerPath,
but forwards CLI arguments verbatim: the in-container daemon receives
`--workspace C:\qwen-repro` in host shape. On POSIX, path.resolve then
treats the whole string as relative and prepends the cwd, so every ACP
child spawn fails with `chdir(2) ENOENT` before running anything — the
container looks healthy (mount and WorkingDir are correct) while every
shell tool call fails (QwenLM#7139).

canonicalizeWorkspace — the choke point every workspace path flows
through (boot argv, client-registered workspaces, persisted
registrations) — now maps Windows-absolute paths to the launcher's
mount convention (`C:\work\proj` → `/c/work/proj`) before resolution.
The translation is deliberately conservative: it only applies on POSIX
inside a container sandbox (SANDBOX env set; seatbelt excluded), only
to `<drive>:\…`-shaped input, and only when the translated candidate
actually exists. Everything else — Windows hosts, unsandboxed runs,
POSIX-shaped paths — is byte-for-byte unchanged.

Verified: unit matrix over the translation guards; a wiring regression
test (fails on the unpatched source) proving canonicalizeWorkspace
resolves `C:\qwen-repro` to `/c/qwen-repro` under a sandbox env; and a
macOS `qwen serve` boot + session-create smoke confirming the normal
path is untouched. The full Windows + Docker Desktop flow needs the
reporter's environment — validation requested on the issue.

Fixes QwenLM#7139

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

Verification report

verification

Wiring regression (workspacePaths.sandbox.test.ts, real canonicalizeWorkspace with SANDBOX env stubbed and the mount-existence probe mocked): on unpatched origin/main the test fails — C:\qwen-repro resolves to <cwd>/C:\qwen-repro, the exact mangled path behind the reported chdir(2) ENOENT; with this PR it resolves to /c/qwen-repro. A second test pins that unsandboxed POSIX behavior is byte-for-byte unchanged.

Guard matrix (translateWindowsWorkspaceForPosixSandbox, seams injected): translation fires only for POSIX + container SANDBOX + Windows-absolute shape + existing target. Inert on: win32 host, no sandbox, sandbox-exec, POSIX/relative/C:-without-separator input, and when the translated mount does not exist. Separator conversion, drive lowercasing, spaces, and nesting covered (D:/Work/proj sub/d/Work/proj sub).

Suite & statics: packages/acp-bridge 864/864 · npm run typecheck ✅ · eslint --max-warnings 0 ✅ · prettier ✅.

Real-daemon smoke (macOS, unsandboxed): qwen serve --workspace <tmp> boots, GET /capabilities responds, POST /session returns {"sessionId": …, "workspaceCwd": "/private/tmp/qwen7139-ws.…"} — the normal path through the changed function is untouched.

Honest limits: the Windows 11 + Docker Desktop end-to-end flow is not reproducible on my hardware; the fix derives from the triage's confirmed source read plus the launcher's own getContainerPath convention, and reporter validation has been requested on #7139. Review focus: (1) the three-condition guard's placement BEFORE path.resolve (resolution itself is what mangles the shape); (2) choke-point coverage — boot argv, client-registered, and persisted workspaces all pass through canonicalizeWorkspace, which is why the host-side entrypoint() argv forwarding is deliberately untouched.

中文版本

接线回归(真实 canonicalizeWorkspace + stub 的 SANDBOX env + mock 的挂载存在性探针):未修复 main 上测试失败——C:\qwen-repro 被解析成 <cwd>/C:\qwen-repro,正是报告中 chdir(2) ENOENT 背后的畸形路径;本 PR 后解析为 /c/qwen-repro。另一测试钉死非沙箱 POSIX 行为逐字节不变。

守卫矩阵:翻译仅在 POSIX + 容器 SANDBOX + Windows 绝对形状 + 目标存在时生效;win32 宿主、无沙箱、sandbox-exec、POSIX/相对/无分隔符输入、翻译目标不存在时全部不生效;分隔符转换、盘符小写、空格与多级目录均覆盖。

套件与静态检查:acp-bridge 864/864;typecheck/eslint/prettier 全绿。

真实 daemon 冒烟(macOS 非沙箱):serve 启动、capabilities 正常、POST /session 返回正确规范化的 workspaceCwd——常规路径不受影响。

如实说明:Windows 11 + Docker Desktop 全链路无法在本机复现;修复基于 triage 已确认的源码分析 + 启动器自身 getContainerPath 约定,已在 #7139 请报告者验证。审阅要点:(1) 三重守卫位于 path.resolve 之前(解析本身就是损坏形状的环节);(2) 收口点覆盖——启动 argv、客户端注册、持久化注册都经过 canonicalizeWorkspace,因此有意不改宿主侧 entrypoint() 的 argv 转发。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug with solid evidence — #7139 is a P1 with a clear reproduction (Windows 11 + Docker Desktop, every shell tool call fails with chdir(2) ENOENT). The mechanism is well-understood: the host-side launcher translates the mount via getContainerPath, but entrypoint() forwards --workspace C:\… verbatim, so the in-container daemon's path.resolve mangles it into a nonexistent relative path. Not theoretical.

Direction: aligned — this fixes a real P1 that completely breaks qwen serve + Docker on Windows. The fix sits at the canonicalization choke point (canonicalizeWorkspace) rather than patching a single entry, which covers --workspace argv, client-registered, and persisted workspace paths. CHANGELOG has no direct reference but the area (sandbox path handling) is clearly relevant.

Size: not applicable — packages/acp-bridge/src/ is not a core module path. Production change is ~50 lines in one file; the rest is tests.

Approach: the scope feels right. The three-condition guard (POSIX container sandbox only, Windows-absolute shape only, translated target must exist) is conservative and mirrors the existing getContainerPath convention exactly. The opts seams for testability are a reasonable pattern. No unrelated changes or drive-by refactors — every edit serves the stated goal.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,证据充分——#7139 是 P1,复现清晰(Windows 11 + Docker Desktop,每个 shell 工具调用都报 chdir(2) ENOENT)。机制明确:宿主侧启动器通过 getContainerPath 翻译了挂载,但 entrypoint() 原样转发 --workspace C:\…,容器内 daemon 的 path.resolve 把它拼成不存在的相对路径。不是理论性问题。

方向:对齐——修复了一个真实 P1,Windows 上 qwen serve + Docker 完全不可用。修复放在规范化收口点(canonicalizeWorkspace)而非单一入口,覆盖了 --workspace 参数、客户端注册和持久化注册路径。

规模:不适用——packages/acp-bridge/src/ 不是核心模块路径。生产代码改动约 50 行,其余为测试。

方案:范围合理。三重守卫(仅 POSIX 容器沙箱、仅 Windows 绝对形状、翻译目标必须存在)保守且与现有 getContainerPath 约定完全一致。没有无关改动或顺手重构。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at 1dd53936ba31f086f0d3653177eb818e368044e2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: I'd add a conservative translation at the canonicalizeWorkspace choke point — detect Windows-absolute paths (<drive>:\…), map them to the container mount convention (/c/…) mirroring getContainerPath, guard with three conditions (POSIX container sandbox only, Windows shape only, translated target exists), and test all guard branches.

Comparison: the PR matches this almost exactly. The implementation is clean:

  • The regex ^([A-Za-z]):[\\/](.*)$ correctly matches Windows absolute paths with both \ and / separators.
  • The translation /${drive.toLowerCase()}/${rest.replace(/\\/g, '/')} mirrors getContainerPath in cli/src/utils/sandbox.ts line-for-line.
  • The three-condition guard is conservative: platform === 'win32' early-return, !sandboxEnv || sandboxEnv === 'sandbox-exec' exclusion (macOS seatbelt doesn't remap), and exists(translated) fallback.
  • The 'sandboxEnv' in opts check is a nice touch — lets tests explicitly pass undefined vs. using the env default.
  • Non-null assertions on match[1]! / match[2]! are safe — the regex guarantees two capture groups.
  • The new existsSync import is the only new dependency, and it's already used elsewhere in the codebase.

No critical blockers. No AGENTS.md violations. The code is focused — every edit serves the stated goal, no drive-by changes.

Unit tests: 8/8 pass (6 in workspacePaths.test.ts, 2 in workspacePaths.sandbox.test.ts). The guard matrix covers: Windows host, no sandbox, seatbelt, non-Windows-shaped input, translated mount missing — all inert. The sandbox wiring test correctly skips on win32.

Real-Scenario Testing

The actual bug only reproduces on Windows 11 + Docker Desktop (the reporter's environment). What I can verify here: the normal POSIX path through canonicalizeWorkspace is unaffected — qwen serve boots, binds the workspace, and creates sessions with the correct workspaceCwd.

$ node .../dist/cli.js serve --workspace /tmp/triage-7228-182128/ws --port 18228
qwen serve: daemon log → .../debug/daemon/daemon.log
qwen serve: Web Shell UI served from .../dist/web-shell
qwen serve listening on http://127.0.0.1:18228 (mode=http-bridge, workspace=/tmp/triage-7228-182128/ws)
qwen serve: bound to workspace "/tmp/triage-7228-182128/ws"
qwen serve: startup timing: processToListenMs=565 runQwenServeToListenMs=415
qwen serve: bearer auth disabled (loopback default). Set QWEN_SERVER_TOKEN to enable.
[INFO] [DAEMON] deferred runtime: scheduling fallback start in 1000ms
[INFO] [DAEMON] deferred runtime: fallback timer fire, starting
[INFO] [DAEMON] ideEnvPresent=false primary=/tmp/triage-7228-182128/ws secondary= daemon workspace roots initialized
qwen serve: session reaper started (interval 60000ms, idle threshold 1800000ms)
qwen serve: /acp WebSocket transport enabled on /acp

$ curl -s http://127.0.0.1:18228/capabilities | python3 -m json.tool | head -5
{
    "v": 1,
    "protocolVersions": { "current": "v1", "supported": ["v1"] },
    "qwenCodeVersion": "0.20.0",
    "mode": "http-bridge",

$ curl -s -X POST http://127.0.0.1:18228/session -H 'Content-Type: application/json' \
    -d '{"workspaceCwd": "/tmp/triage-7228-182128/ws"}'
{
    "sessionId": "2626f5ce-955f-4b9e-8c45-0cae06a2912d",
    "workspaceCwd": "/tmp/triage-7228-182128/ws",
    "attached": false,
    "clientId": "client_e15aaef7-1030-4d1d-a170-056021606fa6",
    "createdAt": "2026-07-19T10:22:00.591Z"
}

Guard behavior verified via direct node invocation:

No sandbox, Windows path: C:\qwen-repro          ← unchanged ✓
With sandbox, mount missing: C:\qwen-repro        ← unchanged ✓
Normal POSIX path: /tmp/triage-7228-182128/ws     ← canonicalized ✓
中文说明

代码审查:实现干净,与独立提案几乎完全一致。正则正确匹配 Windows 绝对路径,翻译逻辑与 getContainerPath 逐行对应,三重守卫保守(win32 早返回、排除 seatbelt、翻译目标必须存在)。无关键阻塞项,无 AGENTS.md 违规。8/8 单测通过。

真实场景测试:实际 bug 仅在 Windows 11 + Docker Desktop 上复现。本地验证了正常 POSIX 路径不受影响——qwen serve 正常启动、绑定工作区、创建 session 返回正确的 workspaceCwd。守卫行为通过 node 直接调用验证:无沙箱时 Windows 路径原样通过,沙箱内挂载不存在时原样通过,正常 POSIX 路径正常规范化。

Qwen Code · qwen3.7-max

Reviewed at 1dd53936ba31f086f0d3653177eb818e368044e2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal fix for a real P1; only nit is the Windows + Docker e2e can't be verified here (author disclosed this honestly).

This is a well-crafted fix. The problem is real and clearly diagnosed — path.resolve('C:\qwen-repro') on Linux prepends the cwd and produces a path that exists nowhere, breaking every shell tool call before it starts. The fix sits at exactly the right place: the canonicalization choke point that every workspace path flows through, not just the --workspace argv entry. The three-condition guard is conservative enough that I can't construct a realistic false-positive scenario — you'd need a Linux directory literally named C:\x inside a sandbox whose /c/x also exists, which the author called out as a deliberate trade.

The code matches my independent proposal almost line-for-line. The translation mirrors getContainerPath exactly, the tests pin every guard branch, and the smoke test confirms the normal POSIX path is byte-for-byte unchanged. If I had to maintain this in six months, I'd thank the author — the JSDoc explains the why, the guards are self-documenting, and the test names read like a spec.

Non-blocking nit: the end-to-end Windows 11 + Docker Desktop flow remains unverified on this hardware. The author disclosed this clearly and requested reporter validation on the issue. The unit tests + wiring regression test (which fails on unpatched source) give good confidence, but a maintainer may want to confirm on the reporter's setup before merge.

中文说明

这是一个精心制作的修复。问题真实且诊断清晰——Linux 上 path.resolve('C:\qwen-repro') 会拼上 cwd 产生不存在的路径,导致每个 shell 工具调用在执行前就失败。修复放在恰好正确的位置:所有 workspace 路径都经过的规范化收口点,而不仅仅是 --workspace 参数入口。三重守卫足够保守,我无法构造出真实的误报场景。

代码与独立提案几乎逐行一致。翻译逻辑与 getContainerPath 完全对应,测试钉死了每个守卫分支,冒烟测试确认正常 POSIX 路径逐字节不变。非阻塞建议:Windows 11 + Docker Desktop 全链路尚未在本机验证,作者已如实披露并在 issue 请报告者验证。维护者可能希望在合并前于报告者环境确认。

Qwen Code · qwen3.7-max

Reviewed at 1dd53936ba31f086f0d3653177eb818e368044e2 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

The workspacePaths.ts sandbox-path translation (QwenLM#7139) reads
process.env.SANDBOX to detect a container-sandboxed daemon; register the
access in the serve process-env allowlist with its rationale — the
sandbox marker is inherently process-scoped (set by the launcher for the
whole relaunched process).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

CI failure addressed in 4ab9f67: the only failing test was the serve process-env guard (process-env-guard.test.ts) flagging the new undocumented process.env.SANDBOX read in workspacePaths.ts. Registered the access in allowedProcessEnvAccesses with its rationale — the sandbox marker is inherently process-scoped (the launcher sets it for the whole relaunched process). Guard test 3/3 locally; no production-code change.

中文:CI 失败已在 4ab9f67 处理——唯一失败项是 serve 的 process.env 守卫测试,标记 workspacePaths.ts 新增的 process.env.SANDBOX 读取未登记。已按守卫要求在 allowedProcessEnvAccesses 登记并说明理由(沙箱标记天然是进程级:由启动器为整个重启进程设置)。守卫测试本地 3/3,无产品代码变更。

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 88a5fc9, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 4 scenario(s).

Qwen Code · serve A/B

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// paths must be mapped to their bind-mount location BEFORE resolution —
// `path.resolve('C:\\x')` on POSIX treats the whole string as relative
// and prepends the cwd.
const resolved = path.resolve(translateWindowsWorkspaceForPosixSandbox(p));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The translation added inside canonicalizeWorkspace is unreachable on the primary workspace-ingestion paths. Every caller validates with path.isAbsolute before calling canonicalizeWorkspace, and on Linux (the container platform) path.isAbsolute('C:\\work\\proj') is false — so the guard throws first and the new translation never runs.

In run-qwen-serve.ts, validateAndCanonicalizeWorkspace throws Invalid --workspace "...": must be an absolute path. at line 2057, before canonicalizeWorkspace at line 2094. (Even past that guard, fs.statSync('C:\\work\\proj') would ENOENT on Linux.) The same isAbsolute-before-canonicalize pattern repeats in dispatch.ts:327 (parseOptionalWorkspaceCwd), bridge.ts:2849 (resolveWorkspaceKey), request-helpers.ts:148, and workspace-management.ts:243.

— Failure scenario: Windows 11 + QWEN_SANDBOX=docker, qwen serve --workspace C:\qwen-repro. The launcher's entrypoint forwards --workspace verbatim into the container; validateAndCanonicalizeWorkspace rejects it as non-absolute before the translation runs, so the daemon fails to boot (and client-registered / persisted workspaces are rejected the same way). The chdir(2) ENOENT bug from #7139 is therefore not fixed for the --workspace boot path. The new wiring test passes only because it calls canonicalizeWorkspace directly, bypassing the guard that exists in the real boot path.

Translate before the absolute-path guard in each caller (or at the sandbox entrypoint/argv boundary), e.g. in validateAndCanonicalizeWorkspace:

const translated = translateWindowsWorkspaceForPosixSandbox(workspace);
if (!path.isAbsolute(translated)) {
  throw new Error(`Invalid --workspace "${workspace}": must be an absolute path.`);
}
// …stat check on `translated`…
return canonicalizeWorkspace(translated);

and analogously in resolveWorkspaceKey, parseOptionalWorkspaceCwd, and the workspace-management route. A wiring test that drives the real boot path (through the isAbsolute guard) — not canonicalizeWorkspace alone — would catch this.

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 3046e3b — the finding was exactly right: I verified validateAndCanonicalizeWorkspace (and the four sibling sites) guard with path.isAbsolute before ever reaching canonicalizeWorkspace, so the translation was unreachable on the real ingestion paths and the wiring test was only exercising the choke point in isolation.

translateWindowsWorkspaceForPosixSandbox is now applied ahead of the guard at each named site: validateAndCanonicalizeWorkspace (boot argv, per your suggested shape — the error messages keep the operator's original input), parseOptionalWorkspaceCwd in both request-helpers.ts and the ACP dispatch.ts, resolveWorkspaceKey in bridge.ts, and the dynamic registration route in workspace-management.ts (its realpathSync.native(resolve(…)) downstream also switched to the translated value). The canonicalizeWorkspace-level translation stays as a defense-in-depth backstop for guard-less callers.

Also added the wiring test you asked for: request-helpers.sandbox.test.ts drives the real exported REST parser through its isAbsolute guard — Windows-shaped cwd + SANDBOX env → returns /c/qwen-repro; no sandbox → still 400; unmounted drive → still 400. It fails when the call-site translation is removed (verified). The mount-existence probe is stubbed via a new _setSandboxMountExistsForTest seam because the translated /c/… target sits at the filesystem root and cross-package node:fs mocks don't reach the acp-bridge binding. acp-bridge 864/864, cli serve suites 807/807.

中文:发现完全正确——已核实五处调用点都在 canonicalizeWorkspace 之前做 isAbsolute 校验,原修复在真实路径上不可达。现已在每个调用点的守卫之前应用翻译(boot argv 按你给的形状实现、REST/ACP 两处 parseOptionalWorkspaceCwd、bridge 的 resolveWorkspaceKey、动态注册路由及其下游 realpath),choke point 内的翻译保留作纵深防御。并按要求补了穿过真实 isAbsolute 守卫的接线测试(移除调用点翻译即失败;挂载探针经新测试缝 stub,因 /c/… 位于文件系统根、跨包 node:fs mock 触不到 acp-bridge 绑定)。acp-bridge 864/864、cli serve 相关 807/807。

zjunothing added a commit to zjunothing/qwen-code that referenced this pull request Jul 19, 2026
…ute-path guards

Review follow-up on QwenLM#7228 (Critical finding): every workspace-ingestion
path validates with path.isAbsolute BEFORE canonicalizeWorkspace, and on
POSIX isAbsolute('C:\...') is false — so a translation living only
inside canonicalizeWorkspace was unreachable on the real paths: the
in-container daemon rejected `--workspace C:\qwen-repro` at boot, and
client-registered / persisted workspaces 400'd, before the translation
ever ran.

translateWindowsWorkspaceForPosixSandbox is now applied ahead of the
guard at each ingestion site — validateAndCanonicalizeWorkspace (boot
argv), parseOptionalWorkspaceCwd in both the REST request helpers and
the ACP dispatch, resolveWorkspaceKey in the bridge, and the dynamic
workspace-registration route — with the canonicalizeWorkspace-level
translation kept as a defense-in-depth backstop. A new wiring test
drives the real exported REST parser through its isAbsolute guard
(fails when the call-site translation is removed); the mount-existence
probe is stubbed via a _setSandboxMountExistsForTest seam because the
translated /c/... target sits at the filesystem root and cross-package
node:fs mocks do not reach the acp-bridge binding.

acp-bridge 864/864; cli serve suites (acp-http, routes, request
helpers, process-env guard) 807/807.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zjunothing added a commit to zjunothing/qwen-code that referenced this pull request Jul 19, 2026
…ute-path guards

Review follow-up on QwenLM#7228 (Critical finding): every workspace-ingestion
path validates with path.isAbsolute BEFORE canonicalizeWorkspace, and on
POSIX isAbsolute('C:\...') is false — so a translation living only
inside canonicalizeWorkspace was unreachable on the real paths: the
in-container daemon rejected `--workspace C:\qwen-repro` at boot, and
client-registered / persisted workspaces 400'd, before the translation
ever ran.

translateWindowsWorkspaceForPosixSandbox is now applied ahead of the
guard at each ingestion site — validateAndCanonicalizeWorkspace (boot
argv), parseOptionalWorkspaceCwd in both the REST request helpers and
the ACP dispatch, resolveWorkspaceKey in the bridge, and the dynamic
workspace-registration route — with the canonicalizeWorkspace-level
translation kept as a defense-in-depth backstop. A new wiring test
drives the real exported REST parser through its isAbsolute guard
(fails when the call-site translation is removed); the mount-existence
probe is stubbed via a _setSandboxMountExistsForTest seam because the
translated /c/... target sits at the filesystem root and cross-package
node:fs mocks do not reach the acp-bridge binding.

acp-bridge 864/864; cli serve suites (acp-http, routes, request
helpers, process-env guard) 807/807.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing
zjunothing force-pushed the fix/7139-serve-sandbox-cwd branch from 6516feb to 96be77a Compare July 19, 2026 12:35
@github-actions

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

…ute-path guards

Review follow-up on QwenLM#7228 (Critical finding): every workspace-ingestion
path validates with path.isAbsolute BEFORE canonicalizeWorkspace, and on
POSIX isAbsolute('C:\...') is false — so a translation living only
inside canonicalizeWorkspace was unreachable on the real paths: the
in-container daemon rejected `--workspace C:\qwen-repro` at boot, and
client-registered / persisted workspaces 400'd, before the translation
ever ran.

translateWindowsWorkspaceForPosixSandbox is now applied ahead of the
guard at each ingestion site — validateAndCanonicalizeWorkspace (boot
argv), parseOptionalWorkspaceCwd in both the REST request helpers and
the ACP dispatch, resolveWorkspaceKey in the bridge, and the dynamic
workspace-registration route — with the canonicalizeWorkspace-level
translation kept as a defense-in-depth backstop. A new wiring test
drives the real exported REST parser through its isAbsolute guard
(fails when the call-site translation is removed); the mount-existence
probe is stubbed via a _setSandboxMountExistsForTest seam because the
translated /c/... target sits at the filesystem root and cross-package
node:fs mocks do not reach the acp-bridge binding.

acp-bridge 864/864; cli serve suites (acp-http, routes, request
helpers, process-env guard) 807/807.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing
zjunothing force-pushed the fix/7139-serve-sandbox-cwd branch from 96be77a to 3046e3b Compare July 19, 2026 12:36
@zjunothing

Copy link
Copy Markdown
Collaborator Author

Critical finding addressed in 3046e3b (per-thread reply posted): the translation now runs BEFORE the path.isAbsolute guard at all five ingestion sites (boot argv validator, REST + ACP parseOptionalWorkspaceCwd, bridge resolveWorkspaceKey, dynamic workspace-registration route), with the choke-point translation kept as backstop. New wiring test drives the real exported REST parser through its guard and fails when the call-site translation is removed. acp-bridge 864/864 · cli serve suites 807/807 · typecheck/eslint/prettier clean.

中文:Critical 已在 3046e3b 落地(已行内回复)——翻译移到五个入口的 isAbsolute 守卫之前,choke point 保留为兜底;新增穿过真实守卫的接线测试(移除调用点翻译即失败)。测试与静态检查全绿。

…s subpath

The serve fast-path bundle guard (check:serve-fast-path-bundle) rejects
pre-listen static reachability of the ACP bridge runtime; importing
translateWindowsWorkspaceForPosixSandbox from the @qwen-code/acp-bridge
barrel pulled bridge.ts and friends into the run-qwen-serve closure.
Switch the new imports to the existing
@qwen-code/acp-bridge/workspacePaths subpath export — the same route
canonicalizeWorkspace already uses in run-qwen-serve.ts. Guard passes
locally ("Startup bundle closure checks passed"); no behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

CI failure fixed in 59f11f0: the fast-path bundle guard (check:serve-fast-path-bundle) flagged that importing the translation from the @qwen-code/acp-bridge barrel pulled the ACP bridge runtime into the pre-listen run-qwen-serve closure. The new imports now use the existing @qwen-code/acp-bridge/workspacePaths subpath export — the same route canonicalizeWorkspace already takes in run-qwen-serve.ts. Guard passes locally ("Startup bundle closure checks passed"); no behavior change, tests unaffected (sandbox wiring 3/3, process-env guard 3/3).

中文:CI 失败已在 59f11f0 修复——fast-path bundle 守卫检测到从 acp-bridge barrel 导入翻译函数会把 ACP bridge 运行时拽进监听前的 run-qwen-serve 静态闭包。新 import 改用既有的 @qwen-code/acp-bridge/workspacePaths 子路径导出(与 canonicalizeWorkspace 同一路径)。守卫本地通过,无行为变化。

…aths import

fast-path.test.ts pinned the exact single-name
`import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths';`
line; adding the sandbox path translation to the same subpath import
(import/no-duplicates forbids a second statement from the same module)
broke the verbatim match. The assertion now checks the guard's intent —
MAX_WORKSPACE_PATH_LENGTH must be imported from the workspacePaths
subpath and the file must not import from the acp-bridge barrel — and
the merged import is formatted canonically.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

Second CI failure fixed in b646a41 — the only failing test (1/14224) was fast-path.test.ts's import-boundary pin, which asserted the verbatim single-name import { MAX_WORKSPACE_PATH_LENGTH } …/workspacePaths line; merging the sandbox translation into that import (import/no-duplicates forbids a second statement from the same module) broke the exact-string match. The assertion now checks the boundary's intent instead: MAX_WORKSPACE_PATH_LENGTH must come from the workspacePaths subpath AND the file must not import from the acp-bridge barrel (strictly stronger than before). fast-path suite + sandbox wiring 71/71, typecheck/eslint/prettier clean.

中文:第二个 CI 失败已在 b646a41 修复——唯一失败(1/14224)是 fast-path 的 import 边界钉:它逐字断言单名 import 行,而把翻译函数并入同一 subpath import(import/no-duplicates 禁止同模块第二条语句)破坏了精确匹配。断言改为按边界意图检查:MAX_WORKSPACE_PATH_LENGTH 必须来自 workspacePaths 子路径且文件不得从 acp-bridge barrel 导入(比原断言更严格)。71/71,静态检查全绿。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

— qwen3.8-max-preview via Qwen Code /review

Comment on lines +37 to +40
it.skipIf(process.platform === 'win32')(
'keeps Windows-shaped input untouched outside a sandbox',
() => {
const result = canonicalizeWorkspace('C:\\qwen-repro');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This "outside a sandbox" test never stubs SANDBOX, but the file-level vi.mock('node:fs') makes existsSync('/c/qwen-repro') return true for every test in the file. If SANDBOX happens to be set in the environment — e.g. running this suite inside the project's Docker sandbox container, where the launcher sets SANDBOX=qwen-code-sandbox-0 — the translation fires (truthy sandboxEnv, regex match, mocked-true mount probe) and returns /c/qwen-repro, so result.endsWith('C:\\qwen-repro') is false and the test fails spuriously. afterEach(vi.unstubAllEnvs) only restores variables a prior test stubbed; it does not unset a real inherited SANDBOX. — Concrete cost: a flaky, misleading failure that looks like a translation bug but is a test-isolation gap.

Suggested change
it.skipIf(process.platform === 'win32')(
'keeps Windows-shaped input untouched outside a sandbox',
() => {
const result = canonicalizeWorkspace('C:\\qwen-repro');
it.skipIf(process.platform === 'win32')(
'keeps Windows-shaped input untouched outside a sandbox',
() => {
vi.stubEnv('SANDBOX', '');
const result = canonicalizeWorkspace('C:\\qwen-repro');

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 60705e1 — the outside-a-sandbox tests (both this file and the REST-helper sibling) now stub SANDBOX to '' explicitly, exactly for the inherited-env case you describe. 中文:两处 outside-a-sandbox 测试都显式 stubEnv('SANDBOX',''),覆盖套件本身跑在项目 Docker 沙箱内继承真实 SANDBOX 的情形。

Comment on lines +324 to +326
// #7139: map a Windows-shaped cwd to its container bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const sandboxCwd = translateWindowsWorkspaceForPosixSandbox(cwd);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Part of a pattern: only the exported parseOptionalWorkspaceCwd in request-helpers.ts has a sandbox-translation test (request-helpers.sandbox.test.ts). This dispatch-side local parseOptionalWorkspaceCwd — the entry point for all three ACP JSON-RPC cwd call sites — has none. — Failure scenario: a future refactor drops the translateWindowsWorkspaceForPosixSandbox call here; a Windows client sending session/new with cwd: "C:\\project" through the ACP dispatch path inside a Docker sandbox gets AcpParamError: 'cwd' must be an absolute path (the exact #7139 failure) while the REST-route test stays green because it covers a different function. The regression ships undetected. — Suggested fix: add a sandbox-scoped test driving this path with a Windows-shaped cwd, a stubbed SANDBOX env, and the _setSandboxMountExistsForTest seam. (Same gap at workspace-management.ts POST /workspaces and run-qwen-serve.ts validateAndCanonicalizeWorkspace.)

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 60705e1parseOptionalWorkspaceCwd is exported from the dispatch module (with a comment noting why) and dispatch.sandbox.test.ts drives it through its guard: Windows cwd + SANDBOX + mount seam → /c/qwen-repro; no sandbox → the exact AcpParamError you quoted. 中文:dispatch 本地 parser 已导出并新增接线测试(正向翻译成功 / 无沙箱抛 AcpParamError)。

Comment on lines +243 to +245
// #7139: map a Windows-shaped cwd to its container bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const sandboxCwd = translateWindowsWorkspaceForPosixSandbox(cwd);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Part of a pattern: this POST /workspaces route's inline sandbox translation has no test coverage — workspace-management.test.ts has zero Windows-shaped/SANDBOX cases; only request-helpers.ts's parser is covered. — Failure scenario: a future refactor drops the translateWindowsWorkspaceForPosixSandbox(cwd) call; inside a Linux container sandbox a Windows client registering {"cwd": "C:\\project"} hits isAbsolute('C:\\project')false on POSIX and gets a 400 invalid_path instead of /c/project, regressing #7139 for the registration path undetected. — Suggested fix: add a sandbox-translation test (stub SANDBOX + the mount-existence seam, POST a Windows-shaped cwd, assert the translated path is registered). (Same gap at dispatch.ts and run-qwen-serve.ts.)

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 60705e1 — two supertest cases on the real POST /workspaces route: with SANDBOX + the mount seam, the 400 moves from the isAbsolute rejection to the deeper 'Path does not exist' existence check (the root-level translated mount cannot exist in a test env — the error-message shift is the proof the translation ran and cleared the guard); without a sandbox, the original absolute-path 400 is pinned. 中文:真实路由上两条 supertest 用例——沙箱下 400 从 isAbsolute 拒绝移动到更深的存在性检查(错误信息位移即翻译已生效的证明);无沙箱时钉住原 400。

Comment on lines 2061 to 2062
const workspace = translateWindowsWorkspaceForPosixSandbox(rawWorkspace);
if (!path.isAbsolute(workspace)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Part of a pattern: the validateAndCanonicalizeWorkspace boot path — the exact qwen serve --workspace C:\… scenario from the #7139 title — has no sandbox-translation test (run-qwen-serve.test.ts has zero Windows/SANDBOX cases); only request-helpers.ts's parser is covered. — Failure scenario: a future refactor drops or relocates the translateWindowsWorkspaceForPosixSandbox(rawWorkspace) call; a Windows host launching into a Linux Docker sandbox fails boot with Invalid --workspace "C:\proj": must be an absolute path, regressing #7139's primary reproduction undetected. — Suggested fix: add a test that stubs SANDBOX + the mount-existence seam, passes a Windows-shaped --workspace, and asserts the server binds the translated mount path. (Same gap at dispatch.ts and workspace-management.ts.)

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 60705e1 — the boot validator is extracted from the runQwenServe closure to the module-scope validateAndCanonicalizeWorkspaceInput export (behavior unchanged; the closure now aliases it), and run-qwen-serve.sandbox.test.ts covers the primary #7139 reproduction end to end: with SANDBOX + the mount seam and statSync mocked for the root-level mount, --workspace C:\qwen-repro returns /c/qwen-repro (canonicalizeWorkspace's documented ENOENT fallback yields the resolved mount, matching a real container); without a sandbox it still throws 'must be an absolute path'. 中文:boot 校验器提取为模块级导出(行为不变),新测试端到端覆盖 #7139 的主复现路径——沙箱下返回 /c/qwen-repro,无沙箱仍抛 must-be-absolute。

Comment on lines +63 to +64
const translated = `/${match[1]!.toLowerCase()}/${match[2]!.replace(/\\/g, '/')}`;
return exists(translated) ? translated : p;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The exists probe here is documented (and described in the PR) as guaranteeing "the translated candidate actually exists (i.e. the drive really is mounted)", but .. segments defeat that contract: C:\..\..\..\etc matches the regex, producing translated = '/c/../../../etc', and existsSync('/c/../../../etc') resolves .. through the kernel to /etc and returns true — so the function returns a path path.resolve normalizes to /etc, outside the /c/ mount. This is not a privilege escalation (the downstream WorkspaceMismatchError in bridge.ts rejects it, and a client could send /etc directly anyway), but the guard does not validate what it claims for ..-laden input. — Suggested fix: resolve the candidate and confirm it stays under the drive mount before the existence check (returning the untranslated form preserves downstream behavior).

Suggested change
const translated = `/${match[1]!.toLowerCase()}/${match[2]!.replace(/\\/g, '/')}`;
return exists(translated) ? translated : p;
const translated = `/${match[1]!.toLowerCase()}/${match[2]!.replace(/\\/g, '/')}`;
const resolvedTranslated = path.resolve(translated);
if (!resolvedTranslated.startsWith(`/${match[1]!.toLowerCase()}/`)) return p;
return exists(resolvedTranslated) ? translated : p;

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 60705e1 — the candidate is resolved and must stay under its drive prefix before the existence probe runs; C:\..\..\etc now passes through untranslated while in-mount .. (C:\work\..\proj) still translates. Pinned by two new tests. 中文:候选路径先 resolve 并校验仍在盘符前缀内才走存在性探针;越界 .. 原样返回、盘内 .. 正常翻译,两条新测试钉住。

…cover every ingestion site

Review follow-up on QwenLM#7228 (second inline round, five findings):

- The translation's existence probe let `..` segments defeat its
  "translated candidate actually exists" contract (C:\..\..\etc ->
  existsSync('/c/../../etc') resolves to /etc and returns true). The
  candidate is now resolved and must stay under its drive prefix, else
  the input passes through untranslated. Pinned by tests (escape refused,
  in-mount .. still allowed).
- The "outside a sandbox" tests explicitly stub SANDBOX='' — running the
  suite inside the project's own Docker sandbox inherits a real SANDBOX
  that unstubAllEnvs would not remove.
- Every ingestion site now has a sandbox-translation wiring test, not
  just the REST helper: the ACP dispatch parser (exported for the test),
  the boot validator (extracted from the runQwenServe closure to the
  module-scope validateAndCanonicalizeWorkspaceInput export; statSync
  mocked so the full boot path returns the bind mount), and the POST
  /workspaces route (asserts the 400 moves from the isAbsolute rejection
  to the deeper existence check once translation runs).

acp-bridge 865/865; cli serve suites 326/326; fast-path bundle guard
passes ("Startup bundle closure checks passed").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

All five inline findings from the latest round addressed in 60705e1 (per-thread replies posted): ..-escape refusal in the translation's existence contract (+2 tests), explicit SANDBOX='' stubbing for inherited-env isolation, and sandbox-translation wiring tests for every remaining ingestion site — ACP dispatch parser (exported), boot validator (extracted to validateAndCanonicalizeWorkspaceInput, full-path test for the primary #7139 reproduction), and the POST /workspaces route (error-shift assertion through the real supertest harness). acp-bridge 865/865 · cli serve suites 326/326 · fast-path bundle guard passes · typecheck/eslint/prettier clean.

中文:最新一轮五条行内发现已全部在 60705e1 落地(已逐 thread 回复):翻译存在性契约的 .. 越界拒绝(+2 测试)、SANDBOX='' 显式隔离、以及其余全部入口的接线测试(ACP dispatch 导出直测、boot 校验器提取后端到端覆盖 #7139 主复现路径、POST /workspaces 真实路由错误位移断言)。测试与守卫全绿。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline. 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

Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment on lines +2851 to +2856
const resolveWorkspaceKey = (rawWorkspaceCwd: string): string => {
// #7139: host-shaped Windows paths reach the in-container bridge via
// clients and persisted registrations; map to the bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const workspaceCwd =
translateWindowsWorkspaceForPosixSandbox(rawWorkspaceCwd);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] resolveWorkspaceKey is the only one of the five workspace-ingestion sites without a dedicated sandbox-translation wiring test — dispatch.ts, request-helpers.ts, workspace-management.ts, and run-qwen-serve.ts each have one that drives the real ingestion path through its isAbsolute guard. — Failure scenario: if a future refactor removes or reorders the translateWindowsWorkspaceForPosixSandbox call here, a Windows-shaped workspaceCwd arriving via spawnOrAttach/restoreSession would hit the isAbsolute rejection and throw workspaceCwd must be an absolute path — the exact #7139 symptom — with no test to catch it, while the other four sites fail loudly.

Suggested change
const resolveWorkspaceKey = (rawWorkspaceCwd: string): string => {
// #7139: host-shaped Windows paths reach the in-container bridge via
// clients and persisted registrations; map to the bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const workspaceCwd =
translateWindowsWorkspaceForPosixSandbox(rawWorkspaceCwd);
const resolveWorkspaceKey = (rawWorkspaceCwd: string): string => {
// #7139: host-shaped Windows paths reach the in-container bridge via
// clients and persisted registrations; map to the bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const workspaceCwd =
translateWindowsWorkspaceForPosixSandbox(rawWorkspaceCwd);

Suggested fix: add a bridge.sandbox.test.ts that stubs SANDBOX + the mount-existence seam and drives spawnOrAttach({ workspaceCwd: 'C:\\qwen-repro' }) with boundWorkspace /c/qwen-repro, asserting it resolves rather than throwing the absolute-path error.

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in e9c3b00bridge.sandbox.test.ts drives the real spawnOrAttach (fake channel harness) with workspaceCwd: 'C:\\qwen-repro' and boundWorkspace: '/c/qwen-repro': inside a sandbox (stubbed SANDBOX + mount seam) the session resolves; outside one the exact workspaceCwd must be an absolute path rejection is pinned. All five ingestion sites now have wiring coverage. 中文:已补第五个入口的接线测试——真实 spawnOrAttach 经 resolveWorkspaceKey 守卫,沙箱内解析成功、沙箱外钉住绝对路径拒绝。五个入口全部覆盖。

Comment on lines +243 to +246
// #7139: map a Windows-shaped cwd to its container bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const sandboxCwd = translateWindowsWorkspaceForPosixSandbox(cwd);
if (!isAbsolute(sandboxCwd)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The sandbox translation (which internally calls existsSync) runs before the MAX_WORKSPACE_PATH_LENGTH guard below, contradicting that guard's comment ("Bound the input before any filesystem work") and the ordering in the sibling routes — dispatch.ts and request-helpers.ts both check length before translating. — Failure scenario: a client POSTs /workspaces with a Windows-shaped cwd over 4096 chars; the translation builds the string, calls path.resolve, and issues an existsSync (fails ENAMETOOLONG) before the length guard rejects the request — one wasted stat syscall per oversized request that the guard exists to prevent.

Suggested change
// #7139: map a Windows-shaped cwd to its container bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const sandboxCwd = translateWindowsWorkspaceForPosixSandbox(cwd);
if (!isAbsolute(sandboxCwd)) {
if (cwd.length > MAX_WORKSPACE_PATH_LENGTH) {
res.status(400).json({
error: `\`cwd\` exceeds the ${MAX_WORKSPACE_PATH_LENGTH}-character limit`,
code: 'invalid_path',
});
return;
}
// #7139: map a Windows-shaped cwd to its container bind mount before
// the absolute-path guard (no-op outside a POSIX container sandbox).
const sandboxCwd = translateWindowsWorkspaceForPosixSandbox(cwd);
if (!isAbsolute(sandboxCwd)) {

Suggested fix: move the MAX_WORKSPACE_PATH_LENGTH check above the translateWindowsWorkspaceForPosixSandbox call, matching the sibling routes.

— qwen3.8-max-preview via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e9c3b00 — the MAX_WORKSPACE_PATH_LENGTH check moved back above the translation (whose existence probe is indeed a filesystem call), restoring the guard's "before any filesystem work" contract and matching the sibling routes' ordering. 中文:length 守卫已移回翻译之前(翻译的存在性探针确是文件系统调用),与守卫注释契约及兄弟路由顺序一致。

…e bridge site

Review follow-up on QwenLM#7228 (third inline round): the POST /workspaces
length guard moves back above the sandbox translation — the translation's
existence probe is a filesystem call, and the guard's contract ("bound
the input before any filesystem work") plus the sibling routes' ordering
both put the length check first. And the fifth ingestion site gets its
wiring test: bridge.sandbox.test.ts drives spawnOrAttach through
resolveWorkspaceKey's isAbsolute guard with a Windows-shaped
workspaceCwd (translated inside a sandbox; the absolute-path rejection
pinned outside one).

acp-bridge 867/867; workspace-management route 63/63.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

Both findings from the latest round addressed in e9c3b00 (per-thread replies posted): length guard restored above the translation's filesystem probe, and the fifth ingestion site (bridge resolveWorkspaceKey via real spawnOrAttach) now has its sandbox wiring test — all five sites covered. acp-bridge 867/867 · route 63/63 · typecheck/eslint/prettier clean.

中文:最新两条发现已在 e9c3b00 落地(已逐 thread 回复):length 守卫移回翻译的文件系统探针之前;第五个入口(bridge resolveWorkspaceKey,经真实 spawnOrAttach)补上接线测试——五个入口全覆盖。测试与静态检查全绿。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review paused — model quota exhausted. Qwen review stopped: the model API quota is exhausted (reset at 07-20 07:32:00 UTC.). Transient errors auto-retry, but a quota reset is too far out to wait on a runner. Re-run once it resets by commenting @qwen-code /review. See workflow logs.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/acp-bridge/src/bridge.ts Outdated
Comment on lines 2855 to 2857
const workspaceCwd =
translateWindowsWorkspaceForPosixSandbox(rawWorkspaceCwd);
if (!path.isAbsolute(workspaceCwd)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The translate-then-isAbsolute pattern is copy-pasted at all five workspace-ingestion sites (bridge.ts, dispatch.ts, request-helpers.ts, workspace-management.ts, run-qwen-serve.ts) with no shared helper enforcing the ordering. The defense-in-depth inside canonicalizeWorkspace (which also calls translateWindowsWorkspaceForPosixSandbox) is unreachable on these paths because the isAbsolute guard rejects untranslated Windows paths before canonicalizeWorkspace is ever called.

— Failure scenario: a future maintainer adds a sixth workspace-ingestion endpoint following the established pattern of path.isAbsolutecanonicalizeWorkspace, forgetting to call translateWindowsWorkspaceForPosixSandbox first. On Windows + Docker sandbox, the new endpoint rejects every Windows-shaped workspace path — reproducing bug #7139 exactly.

Consider extracting a shared helper (e.g. translateAndAssertAbsolute(rawCwd: string): string) that combines the translation and the absolute-path check, replacing the five duplicated blocks.

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 3675d96 — the ordering now has a single enforcement point: translateAndCheckAbsoluteWorkspacePath in workspacePaths.ts (translate, then platform-aware isAbsolute; returns the translated path or null), and all five ingestion sites consume it while keeping their own error surfaces (boot Error, AcpParamError, the two HTTP 400 shapes). A sixth endpoint now has one obvious call to make and cannot get the ordering wrong. Behavior unchanged — all five wiring tests pass untouched, plus a new unit matrix for the helper. acp-bridge 870/870, fast-path bundle guard passes.

中文:已在 3675d96 落地——顺序收敛到唯一强制点 translateAndCheckAbsoluteWorkspacePath(先翻译后平台感知 isAbsolute,返回翻译路径或 null),五个入口全部改用它并保留各自错误面。第六个入口只需调用一个函数,不可能再弄错顺序。行为不变:五个接线测试原样通过,另加 helper 单测矩阵;fast-path 守卫通过。

…tion sites

Review follow-up on QwenLM#7228 (fourth inline round): the five workspace
ingestion sites each hand-rolled the translate-then-isAbsolute pair, so
a future sixth endpoint following the older isAbsolute-then-canonicalize
pattern would silently reproduce QwenLM#7139. The ordering now lives in one
place — translateAndCheckAbsoluteWorkspacePath in workspacePaths.ts
returns the translated path or null when not absolute — and all five
sites consume it while keeping their own error surfaces (boot Error,
AcpParamError, HTTP 400s). Behavior is unchanged: all five wiring tests
pass untouched, plus a unit matrix for the helper itself.

acp-bridge 870/870; cli serve sandbox suites green; fast-path bundle
guard passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zjunothing

Copy link
Copy Markdown
Collaborator Author

Latest suggestion addressed in 3675d96 (per-thread reply posted): the translate-then-check ordering is now enforced by a single shared helper consumed by all five ingestion sites — see the inline thread for details. acp-bridge 870/870 · serve sandbox suites green · fast-path guard passes · typecheck/eslint/prettier clean.

中文:最新建议已在 3675d96 落地(已行内回复)——translate→isAbsolute 顺序收敛到唯一共享 helper,五个入口统一消费。测试与守卫全绿。

Comment on lines +1716 to +1718
throw new Error(
`Invalid --workspace "${workspace}": must be an absolute path.`,
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Error message interpolates workspace (which is null here) instead of rawWorkspace, producing Invalid --workspace "null": must be an absolute path. — Failure scenario: an operator runs qwen serve --workspace relative/path and sees "null" instead of the path they actually typed, making the boot failure unactionable. The pre-refactor code used the parameter workspace which was the raw input; the extraction renamed it to rawWorkspace and reassigned workspace to the translation result, but this template was not updated. Compare with bridge.ts:2859 which correctly uses rawWorkspaceCwd.

Suggested change
throw new Error(
`Invalid --workspace "${workspace}": must be an absolute path.`,
);
throw new Error(
`Invalid --workspace "${rawWorkspace}": must be an absolute path.`,
);

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the rejection now interpolates rawWorkspace, matching bridge.ts's handling; regression test added asserting Invalid --workspace "relative/path" echoes the typed input rather than "null". Good catch on the extraction rename. 中文:已修——插值改为 rawWorkspace(与 bridge.ts 一致),并加回归测试断言错误信息回显操作者输入而非 "null"。

…tion

Review follow-up on QwenLM#7228: the closure extraction renamed the parameter
to rawWorkspace and reassigned workspace to the translation result, but
the rejection template still interpolated workspace — which is null on
this path, so an operator typing `--workspace relative/path` saw
`Invalid --workspace "null"`. The message now echoes the typed input,
pinned by a regression test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Local runtime verification ✅ — a maintainer reproduced #7139 end-to-end

I built the PR locally and verified it on Linux as root, which let me do something the PR description says wasn't possible on the author's hardware: actually create the root-level /c/qwen-repro and /d/Work/proj sub bind mounts, so the translation's existence-probe and the ACP child's real chdir(2) are exercised for real — no mocks. Three layers, all green.

Verdict: the fix does exactly what it claims, the guards are correct, and behavior outside a POSIX container sandbox is byte-for-byte unchanged. LGTM.

① Compiled canonicalizeWorkspace() — BEFORE (main) vs AFTER (PR), real filesystem

I built two workspacePaths.js dist artifacts from the same tree (AFTER = PR head; BEFORE = the fn reverted to merge-base) and drove the real compiled functions inside a simulated POSIX container sandbox (SANDBOX=qwen-code-sandbox-0) against the real mounts:

input reaching the in-container daemon BEFORE (main) AFTER (#7228)
C:\qwen-repro (SANDBOX set) /root/git/qwen-code-x4/C:\qwen-repro /c/qwen-repro
D:/Work/proj sub (SANDBOX set) …/qwen-code-x4/D:/Work/proj sub /d/Work/proj sub
C:\qwen-repro (no SANDBOX) …/C:\qwen-repro …/C:\qwen-reprobyte-for-byte unchanged
C:\no-such-mount (SANDBOX set) mangled unchanged — never invents a path
C:\..\..\etc (SANDBOX set) mangled refused — drive-escape guard ✅
C:\qwen-repro (seatbelt sandbox-exec) mangled inert — macOS excluded ✅
/c/qwen-repro (POSIX absolute) /c/qwen-repro /c/qwen-repro — unchanged ✅

The ..-escape guard is the one worth calling out: existsSync('/c/../../etc') resolves to /etc and returns true, which would make the "translated candidate exists" contract a lie — the PR correctly refuses to translate anything that resolves outside the drive prefix, and the in-mount C:\work\..\proj → /c/work/../proj case is still allowed.

② End-to-end — a real qwen serve daemon inside a simulated POSIX sandbox

This is the reproduction the issue is actually about. I booted the compiled daemon with a Windows-shaped --workspace and SANDBOX=docker (what the launcher sets inside the container), with /c/qwen-repro present on disk:

# AFTER + SANDBOX=docker
$ SANDBOX=docker node packages/cli/dist/index.js serve --port 0 --workspace 'C:\qwen-repro'
qwen serve listening on http://127.0.0.1:35965 (mode=http-bridge, workspace=/c/qwen-repro)
qwen serve: bound to workspace "/c/qwen-repro"

$ curl -X POST /session -d '{"sessionScope":"thread"}'
{"sessionId":"2bb1eb37-…","workspaceCwd":"/c/qwen-repro","attached":false,…}

$ readlink /proc/<acp-child-pid>/cwd     # the chdir(2) that ENOENT'd in #7139
/c/qwen-repro     ← the ACP child chdir'd into the bind mount successfully ✅

The parent process still carries the raw host-shaped arg (--workspace C:\qwen-repro in its cmdline) — proving the daemon received the un-translated path and mapped it internally. The spawned ACP child's working directory is /c/qwen-repro: the exact chdir(2) that failed with No such file or directory in #7139 now succeeds.

Toggling the fix off (same binary, SANDBOX unset → the translation short-circuits, i.e. the identical code path main runs) reproduces the pre-fix wall:

# SANDBOX unset (fix inert == main behavior)
$ node packages/cli/dist/index.js serve --port 0 --workspace 'C:\qwen-repro'
qwen serve: Invalid --workspace "C:\qwen-repro": must be an absolute path.   (daemon dies at boot)

③ Test suites & mutation (teeth)

  • acp-bridge sandbox suites: 14/14 pass (workspacePaths.test.ts + workspacePaths.sandbox.test.ts + bridge.sandbox.test.ts).
  • cli serve suites: all pass (run-qwen-serve.sandbox, acp-http/dispatch.sandbox, server/request-helpers.sandbox, routes/workspace-management 63/63, process-env-guard, fast-path).
  • Mutation (teeth): neutering translateWindowsWorkspaceForPosixSandbox to a no-op fails exactly the 4 translation assertions while the 8 inert-guard cases still pass; rebuilding that mutant into the acp-bridge dist also makes the cross-package run-qwen-serve.sandbox.test.ts wiring test go red — confirming the PR's "fails on the unpatched source" claim across the package boundary.

One "failure" that is actually the fix working 🎯

On my box a single assertion in workspace-management.test.ts flipped from 400 to 201. Root cause: that test assumes "the root-level translated mount cannot exist in a test," but I really created /c/qwen-repro, so the route's own realpathSync.native('/c/qwen-repro') (a separate probe the test's _setSandboxMountExistsForTest override does not cover) succeeded and the workspace registered (201 Created). Removing the fixture → 63/63 pass. So this isn't a defect — it's the REST registration path translating C:\qwen-repro to its bind mount and succeeding end-to-end.

Notes for the merge decision

  • Scope discipline is good: the three-condition guard (POSIX container sandbox only · Windows-absolute shape only · translated target must exist) plus the ..-escape refusal keeps the blast radius to exactly the launcher's own mount convention. The documented residual — a Linux dir literally named C:\x inside a sandbox where /c/x also exists — is an acceptable trade against a P1.
  • The single translateAndCheckAbsoluteWorkspacePath choke point is the right call: all five ingestion sites (boot argv, bridge client/persisted, ACP cwd, REST register, REST cwd) now share the translate-then-isAbsolute ordering, so a sixth endpoint can't silently reforget it.
  • Windows + Docker Desktop end-to-end still can't be exercised here (no such host), same disclosure as the PR — but the container-side mechanism, which is where the fix lives, is now reproduced on a real running daemon.
🇨🇳 中文版:维护者本地运行时验证

本地运行时验证 ✅ —— 维护者已端到端复现 #7139

我在本地构建了该 PR,并以 Linux root 身份验证。这让我能做到 PR 描述中说作者硬件上做不到的事:真实创建根级挂载点 /c/qwen-repro/d/Work/proj sub,因此翻译的存在性探测、以及 ACP 子进程真实的 chdir(2) 都是真跑,无 mock。三层验证,全绿。

结论:修复行为与描述完全一致,守卫正确,非 POSIX 容器沙箱下的行为逐字节不变。同意合并 (LGTM)。

① 编译后的 canonicalizeWorkspace() —— BEFORE(main) vs AFTER(PR),真实文件系统

用同一棵树构建两个 workspacePaths.js 产物(AFTER=PR 头;BEFORE=该函数回退到 merge-base),在模拟 POSIX 容器沙箱(SANDBOX=qwen-code-sandbox-0)中针对真实挂载点驱动真实编译的函数:

到达容器内 daemon 的输入 BEFORE(main) AFTER(#7228)
C:\qwen-repro(SANDBOX 已设) /root/git/qwen-code-x4/C:\qwen-repro /c/qwen-repro
D:/Work/proj sub(SANDBOX 已设) …/D:/Work/proj sub /d/Work/proj sub
C:\qwen-repro(无 SANDBOX) …/C:\qwen-repro …/C:\qwen-repro —— 逐字节不变
C:\no-such-mount(SANDBOX 已设) 被拼坏 原样返回 —— 绝不臆造路径
C:\..\..\etc(SANDBOX 已设) 被拼坏 拒绝翻译 —— 盘符逃逸守卫 ✅
C:\qwen-repro(seatbelt sandbox-exec 被拼坏 不生效 —— 排除 macOS ✅
/c/qwen-repro(POSIX 绝对路径) /c/qwen-repro /c/qwen-repro —— 不变 ✅

值得特别点名的是 .. 逃逸守卫:existsSync('/c/../../etc') 会解析到 /etc 并返回 true,这会让"翻译目标真实存在"的契约变成谎言 —— PR 正确地拒绝翻译任何解析后逃出盘符前缀的路径,同时盘符内的 C:\work\..\proj → /c/work/../proj 仍被允许。

② 端到端 —— 真实 qwen serve daemon 运行在模拟 POSIX 沙箱内

这正是该 issue 的核心复现。我用 Windows 形状的 --workspace + SANDBOX=docker(启动器在容器内设置的值)启动编译后的 daemon,磁盘上 /c/qwen-repro 真实存在:

# AFTER + SANDBOX=docker
$ SANDBOX=docker node packages/cli/dist/index.js serve --port 0 --workspace 'C:\qwen-repro'
qwen serve listening on http://127.0.0.1:35965 (mode=http-bridge, workspace=/c/qwen-repro)
qwen serve: bound to workspace "/c/qwen-repro"
$ curl -X POST /session -d '{"sessionScope":"thread"}'
{"sessionId":"2bb1eb37-…","workspaceCwd":"/c/qwen-repro","attached":false,…}
$ readlink /proc/<acp-child-pid>/cwd     # 即 #7139 中 ENOENT 的那次 chdir(2)
/c/qwen-repro     ← ACP 子进程成功 chdir 进入 bind mount ✅

父进程 cmdline 仍带着原始宿主形状参数(--workspace C:\qwen-repro),证明 daemon 收到的是未翻译的路径、并在内部完成映射。子进程的工作目录是 /c/qwen-repro#7139 中报 No such file or directory 的那次 chdir(2) 现在成功了。

关掉修复(同一个二进制,不设 SANDBOX → 翻译短路,即 main 走的同一条代码路径)即复现修复前的墙:

# 不设 SANDBOX(修复不生效 == main 行为)
$ node packages/cli/dist/index.js serve --port 0 --workspace 'C:\qwen-repro'
qwen serve: Invalid --workspace "C:\qwen-repro": must be an absolute path.   (daemon 启动即退出)

③ 测试套件与变异测试(teeth)

  • acp-bridge sandbox 套件:14/14 通过workspacePaths.test.ts + workspacePaths.sandbox.test.ts + bridge.sandbox.test.ts)。
  • cli serve 套件:全部通过run-qwen-serve.sandboxacp-http/dispatch.sandboxserver/request-helpers.sandboxroutes/workspace-management 63/63、process-env-guardfast-path)。
  • 变异测试(teeth):translateWindowsWorkspaceForPosixSandbox 改成 no-op,恰好让 4 条翻译断言失败、8 条"不生效"守卫用例仍通过;把该变异体重新构建进 acp-bridge dist,还会让跨包run-qwen-serve.sandbox.test.ts 接线测试变红 —— 跨包验证了 PR "在未修复源码上会失败"的说法。

有一处"失败"其实正是修复在生效 🎯

我机器上 workspace-management.test.ts 有一条断言从 400 变成了 201。根因:该测试假设"根级翻译挂载在测试环境里不可能存在",但我真的创建了 /c/qwen-repro,于是该路由自己的 realpathSync.native('/c/qwen-repro')(一个测试的 _setSandboxMountExistsForTest 覆盖不到的独立探测)成功了,workspace 注册成功(201 Created)。移除该 fixture → 63/63 通过。所以这不是缺陷 —— 而是 REST 注册路径把 C:\qwen-repro 翻译到 bind mount 并端到端成功的体现。

供合并决策参考

  • 范围控制得当:三重守卫(仅 POSIX 容器沙箱 · 仅 Windows 绝对形状 · 翻译目标必须存在)加上 .. 逃逸拒绝,把影响面精确限定在启动器自身的挂载约定内。文档中记录的残留情形(沙箱内字面命名为 C:\x 的 Linux 目录且恰好 /c/x 也存在会被重定向)是对 P1 的可接受取舍。
  • 单一 translateAndCheckAbsoluteWorkspacePath 收口点是正确选择:五个摄入点(启动 argv、bridge 客户端/持久化、ACP cwd、REST 注册、REST cwd)现在共享"先翻译再 isAbsolute"的顺序,第六个端点无法再悄悄漏掉。
  • Windows + Docker Desktop 全链路仍无法在此环境跑(无此宿主),与 PR 的披露一致 —— 但修复所在的容器侧机制,现已在真实运行的 daemon 上复现。

Verified against PR head 88a5fc907 on Linux (Node v22.22.2). BEFORE/AFTER = same tree, target fn reverted to merge-base bc0e2cd18.

@wenshao
wenshao added this pull request to the merge queue Jul 21, 2026
Merged via the queue into QwenLM:main with commit 2803f82 Jul 21, 2026
69 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.20.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: qwen serve Docker sandbox passes an invalid workspace cwd to ACP shell tools

3 participants