fix(cli): expand windows-style tilde paths - #5298
Conversation
| expandedPath = path.join( | ||
| os.homedir(), | ||
| ...p | ||
| .substring(2) |
There was a problem hiding this comment.
[Suggestion] The ~\ branch uses path.join + split/filter, while the ~/ and %USERPROFILE% branches use string concatenation. This creates two issues:
-
Trailing separator inconsistency:
path.joinstrips trailing separators, butpath.normalize(used by the other branches) preserves them. SoresolvePath('~/foo/')→homedir/foo/butresolvePath('~\\foo\\')→homedir/foo(trailing separator lost). -
Strategy divergence: different construction patterns make the function harder to maintain — a future change to one branch may not be correctly ported to the others.
A simpler approach using replaceAll matches the existing concat pattern and avoids both issues:
| .substring(2) | |
| } else if (p.startsWith('~\\')) { | |
| expandedPath = os.homedir() + p.substring(1).replaceAll('\\', '/'); |
— qwen3.7-max via Qwen Code /review
| it('expands USERPROFILE references case-insensitively', () => { | ||
| expect(resolvePath('%USERPROFILE%\\schemas\\input.json')).toBe( | ||
| path.normalize(`${os.homedir()}\\schemas\\input.json`), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] On POSIX, both sides of this assertion produce a path with literal backslash characters (e.g., /home/user\schemas\input.json), which is not a valid POSIX path — the test validates a broken string. Additionally, the test name claims "case-insensitively" but only tests uppercase %USERPROFILE%, never %userprofile%.
Consider making the expected value platform-conditional or adding a lowercase variant:
it('expands lowercase %userprofile% references', () => {
expect(resolvePath('%userprofile%\\schemas\\input.json')).toBe(
process.platform === 'win32'
? path.join(os.homedir(), 'schemas', 'input.json')
: path.normalize(`${os.homedir()}\\schemas\\input.json`),
);
});— qwen3.7-max via Qwen Code /review
| it('normalizes relative paths without resolving them', () => { | ||
| expect(resolvePath('nested/../schema.json')).toBe( | ||
| path.normalize('schema.json'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Three untested edge cases for the new ~\\ branch:
- Bare
~\\(just two characters) — exercisesfilter(Boolean)with an empty segment array - Mixed separators like
~\\foo/bar\\baz— validates the[/\\\\]+regex handles both styles - Trailing separator
~\\foo\\— would have caught the trailing-separator inconsistency with the~/branch
it('expands bare backslash-tilde to the home directory', () => {
expect(resolvePath('~\\')).toBe(path.normalize(os.homedir()));
});
it('handles mixed separators in Windows-style tilde paths', () => {
expect(resolvePath('~\\foo/bar\\baz')).toBe(
path.join(os.homedir(), 'foo', 'bar', 'baz'),
);
});— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
✅ Maintainer verification — local real-binary testVerified this PR locally with a real build (not just unit tests). Verdict: correct, minimal, well-tested, and safe — good to merge. One benign cross-platform side effect is noted below for awareness; it is not a blocker. Environment
1. Static checks (reproducing the PR test plan)
2. Real-binary end-to-end (the actual fix, driven under tmux)
POSIX control 3. Cross-platform behavior (simulation over
|
| 检查项 | 命令 | 结果 |
|---|---|---|
| 单元测试 | vitest run resolvePath.test.ts jsonSchemaArg.test.ts |
✅ 47/47 通过（7 个新增 + 40 个调用方回归） |
| Lint | eslint resolvePath.ts resolvePath.test.ts |
✅ 干净 |
| 格式 | prettier --check … |
✅ 干净 |
| 空白字符 | git diff --check |
✅ 干净 |
| 类型检查 | npm run typecheck -w packages/cli |
✅ 退出码 0，0 错误 |
说明：PR 描述里提到
Session.ts/server.ts存在「已知的无关类型检查失败」。在当前 PR head 上并未复现——packages/cli类型检查完全通过。
2. 真实二进制端到端测试（在 tmux 中驱动实际修复）
resolvePath 为 --json-schema @<路径> 提供解析，而该参数会在报错信息中原样回显解析后的路径——这是一个确定性的、无需鉴权的探针。我构建了 bundle，并在真实的 tmux PTY 中，用同一个 Windows 风格输入分别跑了 PR 前后的二进制（沙箱化 HOME，使 os.homedir() 完全可控）：
输入 --json-schema '@~\qwen5298\badjson.txt' （badjson.txt 存在，内容是非法 JSON）
旧版（PR 前）: could not read "~\qwen5298\badjson.txt": ENOENT ← '~\' 未展开 → 即为该 bug
新版（PR 5298）: content of "/tmp/pr5298_home/qwen5298/badjson.txt" is not valid JSON
← 已展开，'\'→'/'，文件被找到并读取 ✅
POSIX 对照组 @~/qwen5298/badjson.txt 在两个二进制上完全一致 → 现有行为无回归。缺失文件、目录等输入在新二进制上也都回显出正确展开的 /home/... 路径。
3. 跨平台行为（在 path.win32 / path.posix 上模拟）
我在两种 path 实现下复现了新旧逻辑。在 Windows 上 ~\schemas\input.json → C:\Users\<user>\schemas\input.json ✅。所有既有形式（~、~/…、%USERPROFILE%\…、相对路径、绝对路径）在两个平台上都逐字节不变。
观察项（非阻断）
- POSIX 副作用： 在 Linux/macOS 上，
~\foo以前会被当作字面量保留（反斜杠是合法的文件名字符），现在会展开为$HOME/foo。实际几乎无害——~\本就是 Windows 习惯写法，全平台接受它是合理的——但一个（极端罕见、字面带反斜杠）名为~\foo的文件其解析结果会改变。仅作提示。 - 尾部分隔符不一致：
~/会保留尾部斜杠（$HOME/），而单独的~\会丢弃它（$HOME），因为新分支用了path.join(…, …filter(Boolean))。属于外观差异；尾部分隔符在下游会被规范化掉。 - 未引入新的路径穿越面——
~\..\..与早已支持的~/../..行为一致。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hi @tt-a1i — thanks for the fix! The code change itself looks solid and minimal.
However, the PR body doesn't follow our pull request template. The template requires specific headings that help reviewers quickly understand the change:
## What this PR does— describes the change in prose (your "## Summary" covers similar ground but the heading name matters for our review process)## Why it's needed— the motivation / user-facing problem (missing entirely — this is the key one)## Reviewer Test Planwith sub-sections:### How to verify,### Evidence (Before & After),### Tested on(your "## Test Plan" lists commands but is missing the structured reviewer-facing format)## Risk & Scopeand## Linked Issues
Could you update the PR body to match the template? The "Why it's needed" section is especially important — it helps reviewers understand the user-facing problem without having to infer it from the code.
Once that's done, feel free to re-request review or ping /triage again.
中文说明
你好 @tt-a1i，感谢修复！代码改动本身很精简。
不过 PR 正文没有遵循我们的 PR 模板。模板要求特定标题来帮助审阅者快速理解改动：
## What this PR does— 用文字描述变更内容## Why it's needed— 动机 / 面向用户的问题（目前缺失，这是关键的一项）## Reviewer Test Plan及子章节：### How to verify、### Evidence (Before & After)、### Tested on## Risk & Scope和## Linked Issues
请按模板更新 PR 正文。"Why it's needed" 部分尤为重要——它帮助审阅者在不看代码的情况下理解用户侧的问题。
更新后可以重新请求审阅或再次 /triage。
— Qwen Code · qwen3.7-max
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ — all required sections filled in with both English and Chinese. On direction: this is a straightforward usability fix. Windows users typing On approach: the scope is tight — 8 lines in Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 中英文各节均已填写。 方向：这是一个直接的可用性修复。Windows 用户输入 方案：范围紧凑 — 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe implementation is correct. The new No critical issues. No AGENTS.md violations. The diff is minimal and focused — exactly two files, no scope creep. TestingBefore/After — Windows tilde expansionUnit tests — all 7 passBug reproduced on main (tilde-backslash paths not expanded), fix confirmed with PR applied, all regression tests green. 中文说明代码审查实现正确。新的 无关键问题。无 AGENTS.md 违规。diff 最小化且聚焦——仅两个文件，无越界改动。 测试在主分支上复现了 bug（反斜杠波浪号路径未展开），应用 PR 后修复确认生效，所有回归测试通过。 — Qwen Code · qwen3.7-max |
ReflectionStepping back — this is exactly the kind of PR you want to see from a first-time contributor. A real usability gap (Windows users typing My instinct before reading the diff was to just add Bug confirmed on main, fix confirmed with the patch, all 7 tests pass, no regressions on existing Approving. ✅ 中文说明反思退一步看——这正是一位首次贡献者应该提交的 PR 类型。一个真实的可用性缺口（Windows 用户输入 我在阅读 diff 之前的直觉是在已有的 在主分支上确认了 bug，应用补丁后确认修复生效，全部 7 个测试通过，既有的 批准合并。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
3 participants
What this PR does
Teaches the CLI path resolver (
resolvePath) to expand Windows-style tilde paths. Previously the resolver only expanded%USERPROFILE%…, bare~, and POSIX-style~/…paths; a path that began with a backslash tilde such as~\schemas\input.jsonfell through unexpanded and was treated as a literal relative path, so the home directory was never substituted. The PR adds a branch for inputs starting with~\: it joinsos.homedir()with the remaining segments (splitting on both/and\and dropping empty segments), so~\schemas\input.jsonnow resolves to<home>/schemas/input.json. The existing~/…, bare~, and%USERPROFILE%behavior is unchanged — the new case is appended as anelse ifafter the existing branches. A new unit-test file adds focused coverage for empty input, bare~, POSIX~/, the POSIX trailing-separator case, Windows~\, case-insensitive%USERPROFILE%, and relative-path normalization.Why it's needed
resolvePathis what resolves@-prefixed file arguments to--json-schema(it is called on the path portion inpackages/cli/src/config/config.ts, theresolveJsonSchemaArgflow). On Windows, users naturally type tilde paths with backslashes (e.g.--json-schema @~\schemas\input.json). Before this change the~\prefix was left unexpanded, so the home directory was not substituted and the schema file would not be found at the intended location. Expanding~\…the same way~/…is already expanded makes tilde paths behave consistently for Windows users.Reviewer Test Plan
How to verify
npx vitest run packages/cli/src/utils/resolvePath.test.ts packages/cli/src/config/jsonSchemaArg.test.tsnpx eslint packages/cli/src/utils/resolvePath.ts packages/cli/src/utils/resolvePath.test.tsnpx prettier --check packages/cli/src/utils/resolvePath.ts packages/cli/src/utils/resolvePath.test.tsgit diff --checkresolvePath('~\\schemas\\input.json')returnspath.join(os.homedir(), 'schemas', 'input.json')(home directory expanded), whileresolvePath('~/schemas/input.json'),resolvePath('~'), and%USERPROFILE%\…continue to return exactly what they returned before. The newresolvePath.test.tsasserts each of these.Evidence (Before & After)
N/A — non-visible logic fix (a path-string resolver, no TUI surface); covered by the unit tests above.
Tested on
✅ tested ·⚠️ not tested · N/A
Environment (optional)
Unit tests only (npm workspaces); no special environment required.
Risk & Scope
packages/cli/src/utils/resolvePath.ts. The behavior change is limited to inputs that begin with~\, which previously were not expanded at all; all other inputs (~/…, bare~,%USERPROFILE%, relative, and absolute paths) take the same branches as before.npm run typecheck --workspace=packages/clifailure onupstream/main(insrc/acp-integration/session/Session.tsandsrc/serve/server.ts) is unrelated to this PR, which only touchespackages/cli/src/utils/resolvePath*.Linked Issues
No linked issue. This PR was opened directly against the
resolvePathresolver and does not reference or close an existing issue.中文说明
这个 PR 做了什么
让 CLI 的路径解析器
resolvePath能够展开 Windows 风格的波浪号路径。此前解析器只展开%USERPROFILE%…、单独的~以及 POSIX 风格的~/…；以反斜杠加波浪号开头的路径（例如~\schemas\input.json）会原样落空、被当作普通相对路径处理，主目录不会被替换。本 PR 为以~\开头的输入新增一个分支：把os.homedir()与剩余的路径片段拼接（同时按/和\切分并丢弃空片段），于是~\schemas\input.json现在会解析为<home>/schemas/input.json。原有的~/…、单独的~以及%USERPROFILE%行为保持不变——新分支是追加在已有分支之后的else if。新增的单元测试文件针对空输入、单独的~、POSIX 的~/、POSIX 末尾分隔符的情形、Windows 的~\、大小写不敏感的%USERPROFILE%，以及相对路径的归一化提供了集中覆盖。为什么需要
resolvePath用于解析传给--json-schema的、以@开头的文件参数（在packages/cli/src/config/config.ts的resolveJsonSchemaArg流程中对路径部分调用它）。在 Windows 上，用户很自然地会用反斜杠来写波浪号路径（例如--json-schema @~\schemas\input.json）。在本次修改之前，~\前缀不会被展开，主目录不会被替换，于是 schema 文件无法在预期位置被找到。让~\…像现有的~/…一样被展开，可以让波浪号路径对 Windows 用户表现一致。审阅者测试计划
如何验证
npx vitest run packages/cli/src/utils/resolvePath.test.ts packages/cli/src/config/jsonSchemaArg.test.tsnpx eslint packages/cli/src/utils/resolvePath.ts packages/cli/src/utils/resolvePath.test.tsnpx prettier --check packages/cli/src/utils/resolvePath.ts packages/cli/src/utils/resolvePath.test.tsgit diff --checkresolvePath('~\\schemas\\input.json')返回path.join(os.homedir(), 'schemas', 'input.json')（主目录已展开），而resolvePath('~/schemas/input.json')、resolvePath('~')以及%USERPROFILE%\…仍返回与改动前完全一致的结果。新增的resolvePath.test.ts对以上每一项都做了断言。证据（改动前 & 改动后）
N/A——这是不可见的逻辑修复（一个路径字符串解析器，没有 TUI 界面）；由上述单元测试覆盖。
测试平台
✅ 已测试 ·⚠️ 未测试 · N/A
环境（可选）
仅单元测试（npm workspaces）；无需特殊环境。
风险与范围
packages/cli/src/utils/resolvePath.ts。行为变化仅限于以~\开头的输入——这类输入此前根本不会被展开；其它所有输入（~/…、单独的~、%USERPROFILE%、相对路径与绝对路径）走的分支与之前完全相同。upstream/main上已存在的npm run typecheck --workspace=packages/cli失败（位于src/acp-integration/session/Session.ts和src/serve/server.ts）与本 PR 无关，本 PR 只改动packages/cli/src/utils/resolvePath*。关联的 Issue
无关联 issue。本 PR 直接针对
resolvePath解析器提出，未引用或关闭任何已有 issue。AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.