Skip to content

fix(cli): Support Windows-style tilde paths#6029

Merged
wenshao merged 9 commits into
QwenLM:mainfrom
VectorPeak:fix
Jun 30, 2026
Merged

fix(cli): Support Windows-style tilde paths#6029
wenshao merged 9 commits into
QwenLM:mainfrom
VectorPeak:fix

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What this PR does

This PR makes Windows-style tilde-prefixed paths such as ~\docs resolve from the user's home directory, matching the existing behavior for POSIX-style ~/docs.

Why it's needed

Qwen Code accepts paths from places where Windows users naturally type backslash-separated paths, including slash commands such as /cd and shared path-resolution helpers used by the CLI. The existing tilde expansion recognized ~ and ~/..., but it did not recognize ~\... as the same home-relative form. As a result, ~\docs was parsed as an ordinary relative path whose first directory name was the literal character ~, so from a workspace like <workspace> it resolved as <workspace>\~\docs instead of C:\Users\<user>\docs. This makes the home-directory shortcut depend on the separator character even though both spellings express the same intent for Windows users.

Reviewer Test Plan

How to verify

On Windows, verify that ~\docs resolves to the user's home directory, for example C:\Users\<user>\docs, instead of resolving under the current workspace as <workspace>\~\docs. Also verify that the existing ~/docs behavior remains unchanged.

Evidence (Before & After)

Before: ~\docs resolved as a relative path with a literal ~ directory under the current workspace.

After: ~\docs resolves as a home-relative path, equivalent to ~/docs.

Targeted Windows validation run locally:

cd packages/core
npx vitest run src/utils/paths.test.ts --coverage=false

Result: 106 passed, 12 skipped.

cd packages/cli
npx vitest run src/ui/commands/cdCommand.test.ts -t "Windows-style home-relative" --coverage=false

Result: 1 passed, 18 skipped.

git diff --check

Result: passed.

Targeted WSL validation was also run from an isolated Ubuntu-22.04 copy with a fresh Linux-side npm install, so it did not reuse the Windows-installed node_modules tree:

cd packages/core
npx vitest run src/utils/paths.test.ts --coverage=false

Result: 117 passed, 1 skipped.

cd packages/cli
npx vitest run src/ui/commands/cdCommand.test.ts -t 'Windows-style home-relative' --coverage=false

Result: 1 passed, 18 skipped.

Tested on

OS Status
macOS not tested
Windows tested
Linux tested via WSL Ubuntu-22.04

Environment (optional)

Windows validation used the local Windows checkout. WSL validation used an isolated Linux-side copy with a fresh npm install; directly reusing the Windows-installed node_modules from WSL is not valid here because Rollup's platform-specific optional dependency for Linux is not present in that install.

Risk & Scope

  • Main risk or tradeoff: Low; the change only broadens existing tilde expansion to include the Windows backslash separator.
  • Not validated / out of scope: Full cdCommand test file was not clean in this Windows environment because an existing symlink test fails with EPERM; the targeted new test passes.
  • Breaking changes / migration notes: None expected.

Linked Issues

Fixes #6030.

中文说明

这个 PR 做了什么

这个 PR 让 Windows 风格的波浪号路径,例如 ~\docs,从用户主目录解析,和已有的 POSIX 风格 ~/docs 行为保持一致。

为什么需要

Qwen Code 会从多个入口接收路径输入,包括 /cd 这类 slash command,以及 CLI 里共用的路径解析辅助逻辑。在这些地方,Windows 用户很自然会输入反斜杠分隔的路径。原有波浪号展开逻辑识别 ~~/...,但没有把 ~\... 识别成同一种 home-relative 写法。结果是,~\docs 会被当成普通相对路径,其中第一个目录名是字面量 ~;如果当前工作区是 <workspace>,它会解析成 <workspace>\~\docs,而不是 C:\Users\<user>\docs。这会让“用户主目录快捷写法”取决于分隔符字符,而这两种写法对 Windows 用户表达的是同一个意图。

Reviewer Test Plan

如何验证

在 Windows 上,确认 ~\docs 会解析到用户主目录,例如 C:\Users\<user>\docs,而不是解析到当前工作区下的 <workspace>\~\docs。同时确认已有的 ~/docs 行为没有变化。

证据(Before & After)

修改前:~\docs 会被解析成当前工作区下带有字面量 ~ 目录的相对路径。

修改后:~\docs 会被解析成 home-relative 路径,行为等同于 ~/docs

本地 Windows 针对性验证:

cd packages/core
npx vitest run src/utils/paths.test.ts --coverage=false

结果:106 passed, 12 skipped。

cd packages/cli
npx vitest run src/ui/commands/cdCommand.test.ts -t "Windows-style home-relative" --coverage=false

结果:1 passed, 18 skipped。

git diff --check

结果:通过。

同时在 WSL Ubuntu-22.04 的独立 Linux 侧拷贝中,fresh npm install 后运行了针对性验证,避免复用 Windows 安装出来的 node_modules

cd packages/core
npx vitest run src/utils/paths.test.ts --coverage=false

结果:117 passed, 1 skipped。

cd packages/cli
npx vitest run src/ui/commands/cdCommand.test.ts -t 'Windows-style home-relative' --coverage=false

结果:1 passed, 18 skipped。

Tested on

OS Status
macOS not tested
Windows tested
Linux tested via WSL Ubuntu-22.04

Environment (optional)

Windows 验证使用本地 Windows checkout。WSL 验证使用独立 Linux 侧拷贝并 fresh npm install;这里不能直接从 WSL 复用 Windows 安装出来的 node_modules,因为该依赖树缺少 Rollup 在 Linux 下需要的平台相关 optional dependency。

风险与范围

  • 主要风险或取舍:低;这个改动只是把已有的波浪号展开逻辑扩展到 Windows 反斜杠分隔符。
  • 未验证 / 范围外:当前 Windows 环境中完整 cdCommand 测试文件因为一个既有 symlink 测试出现 EPERM,未能作为完整通过项;新增的针对性测试已通过。
  • 破坏性变更 / 迁移说明:预计没有。

Linked Issues

Fixes #6030

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @VectorPeak!

Template looks good ✓ — all required headings present, bilingual, solid test evidence.

On direction: this is a straightforward fix for a real Windows usability issue. ~\docs not resolving like ~/docs is a clear gap for Windows users who naturally type backslash paths. CHANGELOG shows this area has been touched multiple times (#5298, #5253, #5378) — the tilde expansion logic has grown organically across packages, and consolidating it is overdue.

On approach: the right move. The PR correctly identifies that tilde/%USERPROFILE% expansion is duplicated across three places (cdCommand.ts, directoryCommand.tsx, resolvePath.ts) and consolidates it into core's paths.ts. The new private expandTilde() helper cleanly handles both POSIX and Windows separators without polluting the public API. The %USERPROFILE% refinements (proper separator handling, splitting on both / and \) fix latent edge cases. Scope feels tight — every changed file serves the stated goal.

Moving on to code review and testing. 🔍

中文说明

感谢贡献,@VectorPeak

模板完整 ✓ — 所有必需的标题都在,中英双语,测试证据充分。

方向:这是一个直截了当的修复,解决了 Windows 用户的实际痛点。~\docs 不能像 ~/docs 一样解析,对于自然输入反斜杠路径的 Windows 用户来说是明显的缺陷。CHANGELOG 显示这个领域已经被多次触碰过(#5298#5253#5378)——波浪号展开逻辑在各包中自然增长,整合它的时机已到。

方案:正确的做法。PR 正确识别了 tilde/%USERPROFILE% 展开在三处重复(cdCommand.tsdirectoryCommand.tsxresolvePath.ts),并将其整合到 core 的 paths.ts。新增的私有 expandTilde() 辅助函数干净地处理了 POSIX 和 Windows 分隔符,不会污染公共 API。%USERPROFILE% 的改进(正确的分隔符处理、同时按 /\ 分割)修复了潜在的边界情况。范围紧凑——每个改动的文件都服务于既定目标。

进入代码审查和测试阶段。🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal (before reading the diff): I'd add ~\ support to expandHomeDir in core/paths.ts, then replace the three duplicated inline implementations in cdCommand.ts, directoryCommand.tsx, and resolvePath.ts with calls to the core function. I'd also check that resolvePath in core handles ~\ since it has its own tilde logic.

Comparison: The PR matches this proposal and goes further in a good way — it introduces a private expandTilde() helper that both expandHomeDir and resolvePath share, avoids code duplication within core itself, and also refines %USERPROFILE% handling with proper separator detection. The consolidation is thorough: cdCommand.ts drops its entire inline path resolution in favor of one import, directoryCommand.tsx deletes its local expandHomeDir, and cli/resolvePath.ts becomes a one-liner delegate.

No blocking issues found. A few observations:

  • The expandHomeDir function grew from ~10 to ~30 lines to handle %USERPROFILE% separator edge cases. This complexity is justified — path completion needs trailing separator preservation, and the old string-concatenation approach had bugs with mixed separators.
  • resolvePath in core now calls expandTilde() before path.isAbsolute(). This is correct: after tilde expansion, the result is an absolute path that path.resolve would return as-is.
  • The %USERPROFILE% no-separator fallback (%USERPROFILE%foo${homeDir}foo) preserves the legacy behavior. Not ideal semantics, but not introduced by this PR — existing behavior maintained.

Test Results

All affected test suites pass on Linux:

packages/core — src/utils/paths.test.ts
 ✓ 126 passed | 1 skipped (127)

packages/cli — src/ui/commands/cdCommand.test.ts
 ✓ 20 passed (20)

packages/cli — src/ui/commands/directoryCommand.test.tsx
 ✓ 27 passed (27)

packages/cli — src/utils/resolvePath.test.ts
 ✓ 7 passed (7)

Typecheck: all 7 packages clean (tsc --noEmit)
Terminal captures

Core paths tests

$ cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false

 RUN  v3.2.4 /home/github-runner/actions-runner-6/_work/qwen-code/qwen-code/packages/core

 ✓ src/utils/paths.test.ts (127 tests | 1 skipped) 19ms

 Test Files  1 passed (1)
      Tests  126 passed | 1 skipped (127)
   Start at  21:22:09
   Duration  376ms (transform 105ms, setup 24ms, collect 84ms, tests 19ms, environment 0ms, prepare 73ms)

CLI cdCommand tests

$ cd packages/cli && npx vitest run src/ui/commands/cdCommand.test.ts --coverage=false

 RUN  v3.2.4 /home/github-runner/actions-runner-6/_work/qwen-code/qwen-code/packages/cli

 ✓ src/ui/commands/cdCommand.test.ts (20 tests) 124ms

 Test Files  1 passed (1)
      Tests  20 passed (20)
   Start at  21:24:24
   Duration  12.11s (transform 5.07s, setup 260ms, collect 9.44s, tests 124ms, environment 1.65s, prepare 349ms)

CLI directoryCommand tests

$ npx vitest run src/ui/commands/directoryCommand.test.tsx --coverage=false

 RUN  v3.2.4 /home/github-runner/actions-runner-6/_work/qwen-code/qwen-code/packages/cli

 ✓ src/ui/commands/directoryCommand.test.tsx (27 tests) 30ms

 Test Files  1 passed (1)
      Tests  27 passed (27)
   Start at  21:26:39
   Duration  14.76s (transform 6.90s, setup 530ms, collect 11.93s, tests 30ms, environment 1.53s, prepare 248ms)

CLI resolvePath tests

$ npx vitest run src/utils/resolvePath.test.ts --coverage=false

 RUN  v3.2.4 /home/github-runner/actions-runner-6/_work/qwen-code/qwen-code/packages/cli

 ✓ src/utils/resolvePath.test.ts (7 tests) 4ms

 Test Files  1 passed (1)
      Tests  7 passed (7)
   Start at  21:28:57
   Duration  7.94s (transform 4.50s, setup 227ms, collect 6.81s, tests 4ms, environment 448ms, prepare 152ms)

Typecheck

$ npm run typecheck

> @qwen-code/qwen-code@0.19.3 typecheck
> npm run typecheck --workspaces --if-present

> @qwen-code/acp-bridge@0.19.3 typecheck — tsc --noEmit ✓
> @qwen-code/audio-capture@1.0.0 typecheck — tsc --noEmit ✓
> @qwen-code/chrome-bridge@1.0.0 typecheck — tsc --noEmit ✓
> @qwen-code/qwen-code@0.19.3 typecheck — tsc --noEmit ✓
> @qwen-code/qwen-code-core@0.19.3 typecheck — tsc --noEmit ✓
> @qwen-code/sdk@0.1.8 typecheck — tsc --noEmit ✓
> @qwen-code/webui@0.19.3 typecheck — tsc --noEmit ✓

Note on tmux E2E testing: The core change (~\ tilde expansion) is Windows-specific — on Linux, \ is a literal character in filenames, so before/after behavior is identical for the tilde prefix. The PR author already provided Windows and WSL validation results. The value of this review's testing is verifying that the code consolidation didn't break anything on Linux. All 180 tests across 4 affected suites pass, and typecheck is clean.

中文说明

代码审查

独立方案(阅读 diff 之前):我会在 core/paths.tsexpandHomeDir 中添加 ~\ 支持,然后用 core 函数替换 cdCommand.tsdirectoryCommand.tsxresolvePath.ts 中的三处重复内联实现。我还会检查 core 中的 resolvePath 是否处理了 ~\,因为它有自己的 tilde 逻辑。

对比: PR 与我的方案一致,并且以好的方式更进一步——引入了私有的 expandTilde() 辅助函数供 expandHomeDirresolvePath 共享,避免了 core 内部的代码重复,同时改进了 %USERPROFILE% 处理,加入了正确的分隔符检测。整合很彻底:cdCommand.ts 删除了整个内联路径解析,改为一个 import;directoryCommand.tsx 删除了本地 expandHomeDircli/resolvePath.ts 变成了一行委托。

未发现阻断性问题。

测试结果

所有受影响的测试套件在 Linux 上均通过:core paths 126 通过 1 跳过,cdCommand 20 通过,directoryCommand 27 通过,resolvePath 7 通过。Typecheck 全部 7 个包干净通过。

关于 tmux E2E 测试的说明: 核心变更(~\ 波浪号展开)是 Windows 特有的——在 Linux 上 \ 是文件名中的字面字符,因此修改前后行为相同。PR 作者已提供了 Windows 和 WSL 验证结果。本次审查测试的价值在于验证代码整合没有破坏 Linux 上的任何功能。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Stepping back to look at the whole picture: this PR does exactly what it should. The problem is real — Windows users type ~\docs naturally and it breaks. The fix is minimal in intent (add backslash support to tilde expansion) and the execution is clean: three duplicated implementations collapsed into one well-tested core function.

My independent proposal matched the PR's approach closely, which is a good sign — there's an obvious right way to do this and the author found it. The diff doesn't carry any unrelated changes or scope creep. The %USERPROFILE% refinements are the one addition beyond the stated scope, but they fix real bugs in the existing code (string concatenation instead of path.join for segments) and are well-tested.

All 180 tests across the 4 affected suites pass. Typecheck is clean across all packages. The code is straightforward — expandTilde and expandHomeDir are easy to read, the logic branches are clear, and the test coverage is thorough (including mixed separators, trailing separators, edge cases like ~ alone, empty strings, %USERPROFILE% variants).

If I had to maintain this in six months, I'd thank the author for consolidating three scattered implementations into one place with clear intent.

Approving. ✅

中文说明

退一步看整体:这个 PR 做到了它应该做的。问题是真实的——Windows 用户自然输入 ~\docs 却会出错。修复在意图上是最小化的(为波浪号展开添加反斜杠支持),执行干净:三处重复实现合并为一个经过充分测试的 core 函数。

我的独立方案与 PR 的方法高度一致,这是个好信号——存在一个显而易见的正确做法,而作者找到了它。diff 没有夹带任何无关改动或范围蔓延。%USERPROFILE% 的改进是超出声明范围的唯一添加,但它们修复了现有代码中的真实 bug(用字符串拼接而非 path.join 处理路径段),并且经过了充分测试。

4 个受影响测试套件共 180 个测试全部通过。所有包的 typecheck 干净。代码直截了当——expandTildeexpandHomeDir 易于阅读,逻辑分支清晰,测试覆盖全面(包括混合分隔符、尾随分隔符、单独 ~、空字符串、%USERPROFILE% 变体等边界情况)。

如果六个月后需要维护这段代码,我会感谢作者将三处分散的实现整合到了一个地方,意图清晰。

批准。✅

Qwen Code · qwen3.7-max

@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. ✅

@doudouOUC doudouOUC 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.

[Suggestion] The PR patches 2 of ~16 tilde-expansion sites across the codebase. Other locations (prompts.ts, openaiLogger.ts, rule-parser.ts, text-buffer.ts, desktop path handlers) still lack ~\ support. Consider extracting a shared expandTilde(input: string): string utility that handles ~, ~/, and ~\ in one place, then migrating all call sites — this eliminates the whack-a-mole pattern where every new path-handling site must independently remember to check both separators.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/utils/paths.ts Outdated
@@ -329,7 +329,7 @@ export function resolvePath(

if (relativePath === '~') {
return homeDir;
} else if (relativePath.startsWith('~/')) {
} else if (relativePath.startsWith('~/') || relativePath.startsWith('~\\')) {

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] POSIX backslash normalization bug. On macOS/Linux, path.join(homeDir, relativePath.slice(2)) does NOT convert backslashes — they become literal filename characters. Input ~\src\components resolves to /home/user/src\components (broken) instead of /home/user/src/components. The same bug exists in cdCommand.ts:43.

The codebase already has 6+ correct implementations using .split(/[\/\\]+/) before path.join (e.g., packages/cli/src/utils/resolvePath.ts:19-25, packages/core/src/config/storage.ts:59-67). Adopt the proven pattern:

Suggested change
} else if (relativePath.startsWith('~/') || relativePath.startsWith('~\\')) {
} else if (relativePath.startsWith('~/') || relativePath.startsWith('~\\')) {
return path.join(homeDir, ...relativePath.slice(2).split(/[\/\\]+/).filter(Boolean));

Apply the same fix in cdCommand.ts:43:

if (input.startsWith('~/') || input.startsWith('~\\')) {
  const segments = input.slice(2).split(/[\/\\]+/).filter(Boolean);
  return path.normalize(path.join(os.homedir(), ...segments));
}

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/utils/paths.test.ts Outdated
it('expands Windows-style tilde-prefixed paths to home directory', () => {
const homeDir = os.homedir();
const result = resolvePath('/some/base', '~\\documents\\file.txt');
expect(result).toBe(path.join(homeDir, 'documents\\file.txt'));

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] Test validates broken behavior on POSIX. Both the implementation and path.join(homeDir, 'documents\\file.txt') produce the same malformed path (/home/user/documents\file.txt) on macOS/Linux — a path with a literal backslash in the filename. The test passes on POSIX but for the wrong reason.

A correct assertion would expose the bug by expecting proper path joining:

Suggested change
expect(result).toBe(path.join(homeDir, 'documents\\file.txt'));
expect(result).toBe(path.join(homeDir, 'documents', 'file.txt'));

This assertion will FAIL on POSIX with the current implementation (exposing the bug) and PASS after applying the split(/[\/\\]+/) fix.

— qwen3.7-max via Qwen Code /review

@@ -205,6 +205,23 @@ describe('cdCommand', () => {
});
});

it('resolves a Windows-style home-relative path from the home directory', async () => {

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 test only covers the error path (directory not found). No success test verifies that cd ~\existingDir actually calls relocateWorkingDirectory. Consider adding a success-path test mirroring the existing ../next test pattern:

it('moves to a Windows-style home-relative directory', async () => {
  const homeSub = path.join(os.homedir(), `qwen-cd-ok-${process.pid}`);
  fs.mkdirSync(homeSub);
  try {
    const result = await cdCommand.action?.(context, `~\\qwen-cd-ok-${process.pid}`);
    expect(relocateWorkingDirectory).toHaveBeenCalledWith(homeSub, context);
  } finally {
    fs.rmSync(homeSub, { recursive: true });
  }
});

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. This is covered now by the success-path Windows-style home-relative /cd test, and the targeted cdCommand.test.ts -t "Windows-style home-relative" run passed: 2 passed, 18 skipped.

@wenshao wenshao 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.

[Suggestion] directoryCommand.tsx:30 — The CLI's local expandHomeDir() in directoryCommand.tsx (used by /directory add) still only checks p === '~' || p.startsWith('~/') without ~\. A Windows user typing /directory add ~\Projects would get a literal ~\Projects path instead of home-directory expansion. This is the same class of user this PR targets.

Additionally, a grep for startsWith('~/') across the codebase shows ~16 other tilde-expansion sites that still lack ~\ support (e.g. rule-parser.ts:808 for permission path specifiers, openaiLogger.ts:34, core/prompts.ts:42). Consider extracting a single expandTilde(input): string helper and routing all call sites through it, which would also address the normalization inconsistency noted inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/utils/paths.test.ts Outdated
it('expands Windows-style tilde-prefixed paths to home directory', () => {
const homeDir = os.homedir();
const result = resolvePath('/some/base', '~\\documents\\file.txt');
expect(result).toBe(path.join(homeDir, 'documents\\file.txt'));

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 test encodes a POSIX-incorrect result. On macOS/Linux, path.join(homeDir, 'documents\\file.txt') produces a path with literal backslashes in the filename (e.g. /home/user/documents\file.txt), not a nested directory path. On Windows, path.join normalizes backslashes so the result happens to be correct.

The codebase already has a more robust pattern in channels/base/src/paths.ts and cli/src/utils/resolvePath.ts that splits on both separators before joining:

path.join(homeDir, ...relativePath.slice(2).split(/[/\\]+/).filter(Boolean))

This normalizes separators correctly on all platforms. Consider adopting this pattern in both resolvePath() (paths.ts:332) and resolveCdPath() (cdCommand.ts:44), and updating the test assertion accordingly.

— qwen3.7-max via Qwen Code /review

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Addressed the shared-utility concern in 16ccc97.

What changed:

  • Added exported expandTilde() in packages/core/src/utils/paths.ts for ~, ~/..., and ~\....
  • Kept expandHomeDir() as the compatibility wrapper for %USERPROFILE% plus tilde expansion.
  • Updated core resolvePath(), CLI resolvePath(), /directory add, and /cd to reuse the core path expansion instead of carrying separate startsWith('~/') checks.
  • Added tests for the shared tilde helper and /directory add with a Windows-style home-relative path.

I intentionally kept this scoped to the Qwen core/CLI path-resolution layer rather than migrating every startsWith('~/') occurrence across desktop/channel packages, because several of those are separate package boundaries or path-detection/UI parsing sites rather than direct path expansion. This should remove the duplicated whack-a-mole logic in the user-facing CLI/core path resolvers while keeping the PR reviewable.

Validation:

  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 113 passed, 12 skipped.
  • cd packages/cli && npx vitest run src/utils/resolvePath.test.ts src/ui/commands/directoryCommand.test.tsx --coverage=false passed: 33 passed.
  • cd packages/cli && npx vitest run src/ui/commands/cdCommand.test.ts --coverage=false passed 18 tests, but one pre-existing symlink test could not run in this Windows environment because fs.symlinkSync(...) failed with EPERM: operation not permitted before reaching the code under test.
  • git diff --check passed.

@VectorPeak
VectorPeak requested a review from doudouOUC June 30, 2026 03:21
@VectorPeak

Copy link
Copy Markdown
Contributor Author

Hi @doudouOUC and @wenshao, thank you both very much for the careful review and the helpful suggestions.

I believe the shared tilde-expansion concern has now been addressed in 16ccc97455a4df5f1842bbc81dfc0d04ba379734:

  • Added a shared expandTilde() helper in packages/core/src/utils/paths.ts for ~, ~/..., and ~\....
  • The helper splits on both / and \ before joining, so POSIX platforms do not preserve Windows backslashes as literal filename characters.
  • Updated the core/CLI path resolution paths, /directory add, and /cd to reuse the shared helper instead of keeping separate startsWith('~/') checks.
  • Added/updated tests for the shared helper and Windows-style home-relative paths.

Validation I ran:

  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false
  • cd packages/cli && npx vitest run src/utils/resolvePath.test.ts src/ui/commands/directoryCommand.test.tsx --coverage=false
  • cd packages/cli && npx vitest run src/ui/commands/cdCommand.test.ts --coverage=false (the existing symlink test cannot run in my Windows environment due to EPERM before reaching the code under test)
  • git diff --check

When you have time, could you please take another look? I would really appreciate your re-review.

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /review

@github-actions

Copy link
Copy Markdown
Contributor
_Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/28424692918)._

doudouOUC
doudouOUC previously approved these changes Jun 30, 2026

@doudouOUC doudouOUC 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.

Both prior Critical findings are resolved: (1) expandTilde() now uses the split(/[/\\]+/) pattern for proper cross-platform normalization, (2) tests correctly verify home-directory expansion on both POSIX and Windows. The consolidation of cdCommand, directoryCommand, and resolvePath onto the shared utility is clean. LGTM! ✅

— qwen3.7-max via Qwen Code /review

import {
loadServerHierarchicalMemory,
ConditionalRulesRegistry,
expandHomeDir,

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] getDirPathCompletions (line ~78) still uses inline trimmed.replace(/^~/, os.homedir()) for tilde expansion. This regex doesn't recognize ~\ as home-relative, so tab-completing ~\project produces no suggestions on any platform — even though /cd ~\project and /directory add ~\project now work correctly after this PR.

Since this file was already modified to import expandHomeDir from core, the completion function could use the same shared utility:

Suggested change
expandHomeDir,
import {
expandHomeDir,
expandTilde,
} from '@qwen-code/qwen-code-core';

Then in getDirPathCompletions, replace the inline replace(/^~/, ...) with expandTilde(trimmed).

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch. The completion path now uses the public expandHomeDir(trimmed) helper rather than the inline trimmed.replace(/^~/, os.homedir()), so ~\\... completion follows the same path expansion as /cd and /directory add. Targeted validation after the latest update: directoryCommand.test.tsx passed: 27 passed.

return os.homedir();
}
if (p === '~/' || p === '~\\') {
return os.homedir() + path.sep;

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] Trailing-separator inconsistency: expandTilde('~/') returns homedir + path.sep (with trailing /), but expandHomeDir('~/') wraps the result in path.normalize(), which strips the trailing separator. Core resolvePath calls expandTilde directly without normalizing, so its callers get a path with trailing separator while expandHomeDir callers don't.

This creates a subtle API footgun — path.dirname(expandTilde('~/')) and path.dirname(expandHomeDir('~/')) return different directories. Consider either normalizing in expandTilde as well, or documenting the trailing-separator contract explicitly in both functions' JSDoc.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/utils/paths.test.ts Outdated
});

it('should expand Windows-style ~\\path to home directory path', () => {
expect(expandTilde('~\\documents')).toBe(path.join(homeDir, 'documents'));

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] Test gap: the expandTilde tests don't cover mixed separators (e.g., expandTilde('~/foo\\bar/baz')). The mixed-separator splitting via .split(/[/\\]+/) is the core of the cross-platform fix — if a future refactor changes the regex to split('/') only, no test would catch the regression.

it('should handle mixed separators', () => {
  expect(expandTilde('~/foo\\bar/baz')).toBe(path.join(homeDir, 'foo', 'bar', 'baz'));
});

Also consider a test for resolvePath('/base', '') (empty relativePath), which exercises the expandTilde('')''path.resolve(baseDir, '') path.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/utils/paths.ts Outdated
* @param p - The path to expand.
* @returns The expanded path.
*/
export function expandTilde(p: string): string {

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] expandTilde is exported from utils/paths.ts, and packages/core/src/index.ts re-exports all of utils/paths.js, so this becomes public API. The only production uses are inside this same module (expandHomeDir and resolvePath); external callers continue to use expandHomeDir. Since this is an implementation detail for those helpers, keeping it private avoids adding another home-expansion API surface.

Suggested fix: remove export from expandTilde, and keep tests focused on the public expandHomeDir / resolvePath behavior unless another package needs tilde-only expansion.

— GPT-5 via Qwen Code /review

Comment thread packages/core/src/utils/paths.ts Outdated
if (p === '~/' || p === '~\\') {
return os.homedir() + path.sep;
}
if (p.startsWith('~/') || p.startsWith('~\\')) {

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 branch now splits the suffix on both / and \\ for the existing ~/... form, not just for the new ~\\... form. On POSIX, ~/foo\\bar used to resolve to a filename containing a literal backslash under $HOME, but now resolves to $HOME/foo/bar. The new Windows-style ~\\... support can normalize backslashes without changing how the already-supported ~/... spelling treats literal backslashes.

Suggested fix: branch by prefix so ~/... preserves the old path.join(os.homedir(), p.slice(2)) behavior, and apply the / + \\ segment splitting only to the new ~\\... case.

— GPT-5 via Qwen Code /review

Comment thread packages/core/src/utils/paths.ts Outdated
return '';
}
let expandedPath = p;
if (p.toLowerCase().startsWith('%userprofile%')) {

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] startsWith('%userprofile%') also matches values where the placeholder is not followed by a separator, but the new path.join(...split(...)) treats the remaining text as path segments. That changes legacy inputs such as %USERPROFILE%foo from <home>foo to <home>/foo, and %USERPROFILE%..\\outside can normalize through .. to the parent of the home directory. The segmented join should only run for the exact placeholder or separator-prefixed paths.

Suggested fix: first handle exact %USERPROFILE%, then handle %USERPROFILE%/ and %USERPROFILE%\\ with segment splitting; for other prefixes, preserve the previous literal concatenation/normalization behavior or leave the input unchanged.

— GPT-5 via Qwen Code /review

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Hi @wenshao and @doudouOUC, thank you again for the careful follow-up review.

I pushed another update in 060227c9d5a2c765ffcf08ddefd8d9a4e3dd33f6 to address the latest feedback:

  • Kept expandTilde() private so it does not become a new exported core API surface.
  • Updated getDirPathCompletions() to reuse the public expandHomeDir() path expansion instead of carrying its own inline ~ replacement, so ~\... completion now matches /directory add and /cd behavior.
  • Preserved existing ~/... behavior while applying separator splitting only to the new Windows-style ~\... form.
  • Tightened %USERPROFILE% handling so exact %USERPROFILE% and separator-prefixed paths are handled explicitly, while legacy %USERPROFILE%foo concatenation semantics are preserved.
  • Added tests for mixed separators, empty resolvePath() input, Windows-style completion, and the /cd ~\existingDir success path.

Validation I ran locally:

  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 111 passed, 12 skipped.
  • cd packages/cli && npx vitest run src/ui/commands/directoryCommand.test.tsx --coverage=false passed: 27 passed.
  • cd packages/cli && npx vitest run src/ui/commands/cdCommand.test.ts --coverage=false -t "Windows-style home-relative" passed: 2 passed, 18 skipped.
  • git diff --check passed.

I also reran the full cdCommand.test.ts; the new Windows-style home-relative success test passes, but the full file still hits the pre-existing Windows symlink permission failure (EPERM at the symlink setup in asks again when confirmed path resolves to a different directory) before reaching the relevant code path.

When you have time, could you please take another look? I really appreciate the detailed review.

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Follow-up CI fix pushed in 7e0392e6f3d8b8772bc6e87ce0d001f1555a95e1.

The failing Ubuntu check was caused by resolvePath('~/') losing the trailing separator after the last review update:

Expected: "/home/runner/"
Received: "/home/runner"

This update restores the legacy ~/ / ~\ trailing-separator behavior in the private core tilde helper while keeping the previous review changes intact (expandTilde() remains private, ~/... keeps legacy semantics, and ~\... still normalizes Windows-style separators). I also added public expandHomeDir() coverage for both trailing-separator forms.

Local validation:

  • cd packages/cli && npx vitest run src/utils/resolvePath.test.ts --coverage=false passed: 7 passed.
  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 112 passed, 12 skipped.
  • cd packages/cli && npx vitest run src/ui/commands/directoryCommand.test.tsx --coverage=false passed: 27 passed.
  • git diff --check passed.

@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.

This PR is very close. The core fix — supporting ~\ as a home-relative path — works correctly, and the consolidation into a shared expandTilde() helper is the right architectural move. The before/after comparison clearly shows ~\docs now resolving to $HOME/docs instead of being treated as a relative path.

But there's a real regression: expandHomeDir('~/') drops the trailing slash that the old code preserved. This breaks a pre-existing test on main (keeps the existing POSIX-style trailing separator behavior). The fix is straightforward — expandTilde('~/') needs to preserve the trailing separator, e.g.:

if (p === '~/' || p === '~\\') {
  return os.homedir() + path.sep;
}

Once that's fixed, this is good to ship. Requesting changes for this one issue.

中文说明

这个 PR 非常接近了。核心修复——支持 ~\ 作为 home-relative 路径——工作正常,整合到共享的 expandTilde() 辅助函数是正确的架构选择。修改前后对比清楚地展示了 ~\docs 现在正确解析到 $HOME/docs,而不是被当作相对路径。

但存在一个真实的回归:expandHomeDir('~/') 丢失了旧代码保留的尾部斜杠。这破坏了 main 上一个已有的测试。修复很简单——expandTilde('~/') 需要保留尾部分隔符。

修复后即可合并。针对这一个问题请求修改。

Qwen Code · qwen3.7-max

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Hi @wenshao and @doudouOUC, thank you again for the follow-up review.

The trailing-separator regression called out by the latest automated review has been addressed in the current PR head, 7e0392e6f3d8b8772bc6e87ce0d001f1555a95e1.

Specifically, the private tilde helper now preserves exact home-directory separator inputs:

if (p === '~/' || p === '~\\') {
  return os.homedir() + path.sep;
}

I also added public expandHomeDir() coverage for both forms:

expect(expandHomeDir('~/')).toBe(path.normalize(homeDir + path.sep));
expect(expandHomeDir('~\\')).toBe(path.normalize(homeDir + path.sep));

Local validation after this fix:

  • cd packages/cli && npx vitest run src/utils/resolvePath.test.ts --coverage=false passed: 7 passed.
  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 112 passed, 12 skipped.
  • cd packages/cli && npx vitest run src/ui/commands/directoryCommand.test.tsx --coverage=false passed: 27 passed.
  • git diff --check passed.

The Ubuntu CI job is still running at the moment. Once it completes, could you please take another look? I really appreciate your time and the detailed review.

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Quick update: the GitHub Actions run has completed successfully now.

  • Test (ubuntu-latest, Node 22.x): passed
  • Run tests and generate reports: passed
  • Run no-AK integration smoke tests: passed

So the trailing-separator fix is now covered by both the focused local tests above and the full Ubuntu CI signal. @wenshao @doudouOUC, could you please re-review when convenient? Thank you again.

Comment thread packages/core/src/utils/paths.ts Outdated
...p
.substring(2)
.split(/[/\\]+/)
.filter(Boolean),

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] Trailing separator stripping in the ~\ branch: .split(/[/\\]+/).filter(Boolean) discards empty segments produced by trailing separators, so expandTilde('~\\path\\') returns homedir/path (no trailing sep) while the sibling ~/ branch (path.join(homedir, p.substring(2))) preserves them — expandTilde('~/path/') returns homedir/path/. This asymmetry means ~\docs\ and ~/docs/ produce different shapes after expansion, which can surprise callers that use trailing separators to signal "directory".

Consider detecting and re-attaching a trailing separator:

Suggested change
.filter(Boolean),
const rest = p.substring(2);
const hasTrailingSep = rest.endsWith('/') || rest.endsWith('\\');
const joined = path.join(
os.homedir(),
...rest.split(/[/\\]+/).filter(Boolean),
);
return hasTrailingSep ? joined + path.sep : joined;

The same pattern also appears in the %userprofile%/ branch of expandHomeDir (line 136) and has the same issue — expandHomeDir('%userprofile%/') strips the trailing separator while expandHomeDir('~/') preserves it via expandTilde's explicit guard.

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for calling this out. Addressed in 591c8e5f5: the ~\\... branch now detects a trailing / or \\ before segment splitting and re-attaches path.sep after path.join(...), so ~\\docs\\ keeps the same trailing-directory signal as ~/docs/.

Focused validation: cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 115 passed, 12 skipped.

Comment thread packages/core/src/utils/paths.ts Outdated
...p
.substring(userProfilePrefix.length + 1)
.split(/[/\\]+/)
.filter(Boolean),

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] expandHomeDir('%userprofile%/') strips the trailing separator because p.substring(userProfilePrefix.length + 1) skips the separator character and .split(/[/\\]+/).filter(Boolean) then discards the empty trailing segment. Compare with expandHomeDir('~/') which correctly returns homedir + path.sep via expandTilde's explicit guard.

Add a dedicated early-return for the bare-prefix-with-separator case, mirroring expandTilde's ~/ guard:

Suggested change
.filter(Boolean),
if (lowerPath === `${userProfilePrefix}/` || lowerPath === `${userProfilePrefix}\\`) {
return path.normalize(os.homedir() + path.sep);
}
if (
lowerPath.startsWith(`${userProfilePrefix}/`) ||
lowerPath.startsWith(`${userProfilePrefix}\\`)
) {
const rest = p.substring(userProfilePrefix.length + 1);
const hasTrailingSep = rest.endsWith('/') || rest.endsWith('\\');
const joined = path.join(
os.homedir(),
...rest.split(/[/\\]+/).filter(Boolean),
);
return path.normalize(hasTrailingSep ? joined + path.sep : joined);
}

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed in 591c8e5f5. I added the dedicated %USERPROFILE%/ and %USERPROFILE%\\ early return, and the separator-prefixed %USERPROFILE%/... branch now preserves trailing separators for suffixed paths before normalizing.

This keeps the previous exact %USERPROFILE% and legacy %USERPROFILE%foo behavior intact while making the separator-prefixed forms consistent with ~/ and ~\\.

@@ -946,7 +981,13 @@ describe('expandHomeDir', () => {

it('should expand %userprofile%\\path to home directory path', () => {
const result = expandHomeDir('%userprofile%\\documents');

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 expandHomeDir code has an explicit branch for both %userprofile%/ (forward-slash) and %userprofile%\\ (backslash) — see paths.ts lines 129-130 — but only the backslash variant is tested here. Consider adding a forward-slash test to cover the new branch:

it('should expand %USERPROFILE%/path with forward-slash separator', () => {
  expect(expandHomeDir('%USERPROFILE%/documents')).toBe(
    path.join(homeDir, 'documents'),
  );
});

— qwen3.7-max via Qwen Code /review

@VectorPeak VectorPeak Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added this coverage in 591c8e5f5:

expect(expandHomeDir('%USERPROFILE%/documents')).toBe(
  path.join(homeDir, 'documents'),
);

I also added coverage for %USERPROFILE%/ and %USERPROFILE%\\documents\\ preserving trailing separators, since that was the related behavior gap in the implementation branch.

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Hi @wenshao and @doudouOUC, thank you again for the careful follow-up review.

I pushed 591c8e5f5775d0c5223bb9347a502ab12afe8cf9 to address the latest path trailing-separator feedback:

  • ~\\... expansion now preserves a trailing / or \\ after normalizing Windows-style separators, so ~\\docs\\ keeps the same directory-shaped result as ~/docs/.
  • %USERPROFILE%/ and %USERPROFILE%\\ now have explicit bare-prefix handling, and separator-prefixed %USERPROFILE%/... paths preserve trailing separators for suffixed paths.
  • Added coverage for %USERPROFILE%/documents, ~\\documents\\, %USERPROFILE%/, and %USERPROFILE%\\documents\\.
  • Also replied on the still-visible /cd success-path and directory-completion threads with the corresponding test/implementation notes.

Local validation after this update:

  • cd packages/core && npx vitest run src/utils/paths.test.ts --coverage=false passed: 115 passed, 12 skipped.
  • cd packages/cli && npx vitest run src/utils/resolvePath.test.ts --coverage=false passed: 7 passed.
  • cd packages/cli && npx vitest run src/ui/commands/directoryCommand.test.tsx --coverage=false passed: 27 passed.
  • cd packages/cli && npx vitest run src/ui/commands/cdCommand.test.ts --coverage=false -t "Windows-style home-relative" passed: 2 passed, 18 skipped.
  • git diff --check passed.

GitHub Actions is also green for the current head:

  • Classify PR: passed
  • Test (ubuntu-latest, Node 22.x): passed, including Run tests and generate reports and Run no-AK integration smoke tests

When convenient, could you please take another look? I really appreciate your time and the detailed review.

@doudouOUC

Copy link
Copy Markdown
Collaborator

Thanks for the update. I checked current head 591c8e5f5775d0c5223bb9347a502ab12afe8cf9. The latest trailing-separator and %USERPROFILE% follow-ups look addressed in the touched core/CLI path-expansion scope, and the visible replies on the /cd success-path and directory-completion threads match the current implementation/tests. Current PR checks are green or skipped; Ubuntu passed, while Windows/macOS/integration were skipped for this PR run.

I'm not seeing new actionable feedback from me at this point. The remaining CHANGES_REQUESTED state appears to be from older reviews on previous commits.

中文说明

感谢更新。我看了当前 head 591c8e5f5775d0c5223bb9347a502ab12afe8cf9。最新的尾部分隔符和 %USERPROFILE% 反馈在本 PR 涉及的 core/CLI 路径展开范围内已经处理;/cd 成功路径和目录补全相关线程中的回复也与当前实现和测试一致。当前 PR 检查为通过或跳过状态;Ubuntu 已通过,Windows/macOS/integration 在这次 PR 运行中被跳过。

我这边目前没有看到新的可执行反馈。剩余的 CHANGES_REQUESTED 状态看起来来自旧提交上的历史 review。

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@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. ✅

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Local verification report (maintainer)

I built and tested this PR locally to support the merge decision.

Setup: isolated worktree at head 591c8e5 (clean npm ci + patch-package + full workspace tsc --build — no shared/stale node_modules, so results are free of cross-branch split-brain). Platform: macOS (darwin), Node v22.22.2.

Note on platform: I verified on POSIX. The suites are written platform-agnostically (path.join/path.sep), and on POSIX they still meaningfully exercise the Windows-tilde logic — proven by the mutation test below, where removing the ~\ branch fails the tests on macOS. The only Windows-specific difference is the value of path.sep, which the same code path handles.

Results — all green

Check Result
core paths.test.ts ✅ 126 passed, 1 skipped¹
cli cdCommand.test.ts ✅ 20 passed
cli directoryCommand.test.tsx ✅ 27 passed
cli resolvePath.test.ts ✅ 7 passed
core typecheck (tsc --noEmit) ✅ clean (exit 0)
cli typecheck (tsc --noEmit) ✅ clean (exit 0)
eslint — 8 changed files ✅ clean (exit 0)
git diff --check ✅ clean
Regression: core glob/ripGrep/grep (consume core resolvePath) ✅ 174 passed
Regression: core storage / cli serve/fast-path (use ~\/%userprofile%) ✅ 63 / 58 passed

¹ The 1 skip is the pre-existing unescapePath › should be a no-op on Windows (it.skipIf(!isWindows)), unrelated to this PR — this is also why the WSL/Unix run in the PR description reports 1 skipped.

Mutation testing — the new tests are load-bearing

To confirm the tests actually guard the behavior (not vacuous), I reverted the fix and re-ran:

  • Mutation A — disabled the core Windows-tilde (~\) branch in expandTilde (i.e. the pre-fix behavior): 5 core tests fail (resolvePath, resolveAndValidatePath, and 3× expandHomeDir) and 4 cli tests fail (both cdCommand Windows cases + both directoryCommand Windows cases). This proves both that the new tests are real and that the CLI is genuinely wired through to core (cdCommand → core resolvePath, directoryCommand → core expandHomeDir).
  • Mutation B — reverted %userprofile%\ to old literal-concat: 2 core tests fail. Confirms the %userprofile% backslash-split branch is covered.

Both mutations were reverted; the worktree was byte-identical to the PR head for the final run.

Code review (no blocking issues)

  • os import correctly removed from cdCommand.ts; no remaining file imports the relocated expandHomeDir from directoryCommand (it now lives in core; resolvePath.ts and directoryCommand.tsx import it from there). Both typechecks confirm.
  • All behavior changes are confined to inputs starting with ~\, %userprofile%\, or bare ~/. POSIX ~/… semantics are preserved (the legacy-semantics test guards the case where a path after ~/ contains a literal backslash).
  • cdCommand re-realpaths the resolved target, so the new trailing-separator on bare ~/ has no user-visible effect.

Non-blocking notes for the maintainer

  1. Cross-platform broadening (by design): ~\… and %userprofile%\… now expand on all platforms, not only Windows. On POSIX, \ is a legal filename character, so ~\foo (previously a literal relative path) now expands to $HOME/foo. This is intentional consistency and the CLI-side resolvePath already behaved this way for ~\; the newly-broadened surface is core resolvePath (used by /cd and the glob/grep/ripGrep tools) and core expandHomeDir. Practical risk is negligible.
  2. Cosmetic: core resolvePath('~/') / expandHomeDir('~\') now keep a trailing separator ($HOME/). Harmless (washed out by realpath in /cd; fine as a glob base).
  3. I did not run the esbuild bundle step (skipped to avoid the unrelated daemon-bundle-cap false-negative). Typecheck + full workspace package build passed, and this change is net code-removal with no new deps, so bundle impact is nil.

Verdict

LGTM — correct, well-tested, and safe to merge. Behavior, typecheck, lint, and test coverage all verified locally; mutation testing confirms the suite genuinely guards the fix.

中文版(点击展开)

本地验证报告(维护者)

我在本地构建并测试了该 PR,作为合并决策的参考。

环境: 在 head 591c8e5 上建立独立 worktree(干净的 npm ci + patch-package + 完整 workspace tsc --build,没有复用共享/陈旧的 node_modules,因此结果不受跨分支 split-brain 干扰)。平台:macOS(darwin),Node v22.22.2

关于平台:我在 POSIX 上验证。测试用例本身是平台无关写法(path.join/path.sep),在 POSIX 上依然能真实覆盖 Windows 波浪号逻辑——下面的变异测试证明了这一点:把 ~\ 分支去掉后,这些测试在 macOS 上就会失败。Windows 上唯一的差异是 path.sep 的取值,而同一段代码路径会正确处理它。

结果——全绿

检查项 结果
core paths.test.ts ✅ 126 通过,1 跳过¹
cli cdCommand.test.ts ✅ 20 通过
cli directoryCommand.test.tsx ✅ 27 通过
cli resolvePath.test.ts ✅ 7 通过
core 类型检查(tsc --noEmit ✅ 干净(exit 0)
cli 类型检查(tsc --noEmit ✅ 干净(exit 0)
eslint——8 个改动文件 ✅ 干净(exit 0)
git diff --check ✅ 干净
回归:core glob/ripGrep/grep(消费 core resolvePath ✅ 174 通过
回归:core storage / cli serve/fast-path(用到 ~\/%userprofile% ✅ 63 / 58 通过

¹ 这 1 个跳过是既有的 unescapePath › should be a no-op on Windowsit.skipIf(!isWindows)),和本 PR 无关——这也是为什么 PR 描述里 WSL/Unix 那次运行同样报告 1 个跳过。

变异测试——新测试确实"承重"

为确认这些测试真的在守护行为(而不是空过),我把修复回退后重跑:

  • 变异 A——禁用 core 中 Windows 波浪号(~\)在 expandTilde 里的分支(即修复前的行为):5 个 core 测试失败resolvePathresolveAndValidatePath、以及 3 个 expandHomeDir且 4 个 cli 测试失败cdCommand 两个 Windows 用例 + directoryCommand 两个 Windows 用例)。这同时证明了新测试是真实的,并且 CLI 确实接到了 core 上(cdCommand → core resolvePathdirectoryCommand → core expandHomeDir)。
  • 变异 B——把 %userprofile%\ 回退成旧的字面量拼接:2 个 core 测试失败。证明 %userprofile% 反斜杠拆分分支被覆盖到了。

两个变异都已还原;最终运行时 worktree 与 PR head 逐字节一致。

代码审查(无阻塞问题)

  • cdCommand.ts 中的 os import 已正确移除;没有任何文件再从 directoryCommand 导入被迁走的 expandHomeDir(它现在在 core 里;resolvePath.tsdirectoryCommand.tsx 都从 core 导入)。两边类型检查均确认。
  • 所有行为变化都限定在以 ~\%userprofile%\ 或裸 ~/ 开头的输入上。POSIX 的 ~/… 语义被保留(legacy-semantics 测试守住了"~/ 之后的路径含字面量反斜杠"这种情况)。
  • cdCommand 会对解析后的目标再做一次 realpath,所以裸 ~/ 新增的尾部分隔符对用户不可见。

给维护者的非阻塞说明

  1. 跨平台扩大(有意为之): ~\…%userprofile%\… 现在在所有平台展开,不只是 Windows。在 POSIX 上 \ 是合法文件名字符,因此 ~\foo(以前是字面量相对路径)现在会展开为 $HOME/foo。这是有意的一致性,且 CLI 侧的 resolvePath~\ 本来就是这种行为;这次新扩大的面是 core resolvePath(被 /cd 以及 glob/grep/ripGrep 工具使用)和 core expandHomeDir。实际风险可忽略。
  2. 纯外观: core resolvePath('~/') / expandHomeDir('~\') 现在会保留尾部分隔符($HOME/)。无害(在 /cd 里被 realpath 抹平;作为 glob base 也没问题)。
  3. 没有跑 esbuild bundle 步骤(为避开与本 PR 无关的 daemon-bundle-cap 假失败而跳过)。类型检查 + 完整 workspace 包构建均通过,且本改动是净删代码、无新增依赖,对 bundle 体积影响为零。

结论

LGTM——正确、测试充分、可安全合并。 行为、类型检查、lint、测试覆盖均已本地验证;变异测试确认测试套件真正守护了这个修复。

@wenshao
wenshao added this pull request to the merge queue Jun 30, 2026
Merged via the queue into QwenLM:main with commit f23357d Jun 30, 2026
90 checks passed
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-style tilde paths resolve under the current directory

4 participants