fix(cli): import extension channels via file urls - #5301
Conversation
|
@qwen-code /triage |
✅ Maintainer verification — real
|
| Check | Command | Result |
|---|---|---|
| Unit tests | vitest run …/channel/start.test.ts |
✅ 5/5 (incl. new resolveExtensionChannelEntrySpecifier test) |
| Unit tests | vitest run …/commands/channel |
✅ 32/32 (whole channel dir) |
| Lint | eslint start.ts(.test) |
✅ clean |
| Format | prettier --check |
✅ clean |
| Whitespace | git diff --check |
✅ clean |
| Typecheck | npm run typecheck -w packages/cli |
✅ exit 0, 0 errors |
2. Why this matters (the bug)
Extension channels are loaded with a dynamic import(specifier). The pre-PR code passed a raw filesystem path as the specifier. A raw path is parsed URL-style, so a path segment containing a URL-reserved character (#, ?) is mis-interpreted — and on Windows an absolute path like C:\… is rejected outright (ERR_UNSUPPORTED_ESM_URL_SCHEME). The fix converts the joined path to a proper file:// URL via pathToFileURL(...).href before importing.
3. Real command A/B in tmux
Planted two user extensions (sandboxed HOME) and ran the real qwen channel start demo (which loads every active extension's channel). One entry path contains # (needs URL-encoding); the other is a normal path (control).
OLD (pre-PR):
[Extensions] Failed to load channel "demo" from "demo-ext":
Cannot find module '/tmp/pr5301_home/.qwen/extensions/demo-ext/dist/channel' ← '#v2.mjs' dropped as a URL fragment
[Extensions] Loaded channel "plain" from "plain-ext" ← normal path still OK
NEW (this PR):
[Extensions] Loaded channel "demo" from "demo-ext" ← '#' path imported via file:// URL
[Extensions] Loaded channel "plain" from "plain-ext"
The OLD error is the smoking gun: the module path is truncated at channel — everything from # onward was treated as a URL fragment, so the file was never found. NEW imports it correctly. The normal-path control loads on both binaries → no regression.
4. Function-level confirmation (real exported function)
resolveExtensionChannelEntrySpecifier produces correctly percent-encoded file:// URLs:
extPath + entry |
result |
|---|---|
…/demo-ext + dist/channel#v2.mjs |
file:///…/demo-ext/dist/channel%23v2.mjs (#→%23) |
/tmp/qwen extension + dist/channel.js |
file:///tmp/qwen%20extension/dist/channel.js (空格→%20, the PR's own test case) |
…/plain + dist/channel.mjs |
file:///…/plain/dist/channel.mjs (unchanged) |
Observations (non-blocking)
- Cross-platform impact: on Windows this fixes every extension channel (drive-letter paths can't be a raw ESM specifier). On Linux/macOS it fixes paths with URL-reserved characters — I reproduced the failure with
#on Linux. (A plain space happens to still import on Node 22/Linux, but the fix correctly encodes it anyway, which matters on stricter setups and on Windows.) - Correct order:
path.join(extPath, entry)first (resolve the relative manifest entry against the extension root), thenpathToFileURL(encode the whole absolute path). Entries stay relative to the extension root, as intended. - Scope is minimal and well-tested: the new logic is a small, pure, exported function with direct unit coverage; only the import specifier changed.
🇨🇳 中文版(点击展开)
✅ 维护者验证 —— 在 tmux 中对 qwen channel start 进行真实测试
我在本地通过驱动真实的 qwen channel start 命令加载一个真实的扩展频道进行了验证(外加单测)。结论:这是一个正确且必要的修复,建议合并。 本次补上了 🐧 Linux 覆盖(该 PR 在 macOS 上测试过)。
环境
- 在 PR head(
fc0c68c1,基于当前上游main)上独立git worktree,全新npm ci(绝不软链node_modules),真实 esbuild 打包。 - Linux · Node
v22.22.2· 打包后的qwen0.18.3。 - 改动范围干净:恰好 2 个文件,+31 / −3,单个提交。
1. 静态检查(复现 PR 测试计划)
| 检查项 | 命令 | 结果 |
|---|---|---|
| 单元测试 | vitest run …/channel/start.test.ts |
✅ 5/5(含新增 resolveExtensionChannelEntrySpecifier 测试) |
| 单元测试 | vitest run …/commands/channel |
✅ 32/32(整个 channel 目录) |
| Lint | eslint start.ts(.test) |
✅ 干净 |
| 格式 | prettier --check |
✅ 干净 |
| 空白字符 | git diff --check |
✅ 干净 |
| 类型检查 | npm run typecheck -w packages/cli |
✅ 退出码 0,0 错误 |
2. 为什么重要(这个 bug)
扩展频道是通过动态 import(specifier) 加载的。PR 之前的代码把原始文件系统路径当作 specifier 传入。原始路径会按 URL 方式解析,因此路径中含有 URL 保留字符(#、?)的片段会被错误解析;而在 Windows 上,像 C:\… 这样的绝对路径会被直接拒绝(ERR_UNSUPPORTED_ESM_URL_SCHEME)。本修复在 import 之前通过 pathToFileURL(...).href 把拼好的路径转成规范的 file:// URL。
3. tmux 中真实命令的 A/B
预置了两个用户扩展(沙箱化 HOME),运行真实的 qwen channel start demo(它会加载每个活跃扩展的频道)。一个 entry 路径包含 #(需要 URL 编码),另一个是正常路径(对照)。
旧版(PR 前):
[Extensions] Failed to load channel "demo" from "demo-ext":
Cannot find module '/tmp/pr5301_home/.qwen/extensions/demo-ext/dist/channel' ← '#v2.mjs' 被当作 URL 片段丢弃
[Extensions] Loaded channel "plain" from "plain-ext" ← 正常路径仍然 OK
新版(本 PR):
[Extensions] Loaded channel "demo" from "demo-ext" ← '#' 路径通过 file:// URL 成功导入
[Extensions] Loaded channel "plain" from "plain-ext"
旧版的报错就是铁证:模块路径被截断到 channel——# 之后的部分都被当作 URL 片段,于是文件根本找不到。新版能正确导入。正常路径的对照在两个二进制上都能加载 → 无回归。
4. 函数级确认(真实导出的函数)
resolveExtensionChannelEntrySpecifier 会产出正确百分号编码的 file:// URL:
extPath + entry |
结果 |
|---|---|
…/demo-ext + dist/channel#v2.mjs |
file:///…/demo-ext/dist/channel%23v2.mjs(#→%23) |
/tmp/qwen extension + dist/channel.js |
file:///tmp/qwen%20extension/dist/channel.js(空格→%20,即 PR 自带的测试用例) |
…/plain + dist/channel.mjs |
file:///…/plain/dist/channel.mjs(不变) |
观察项(非阻断)
- 跨平台影响: 在 Windows 上,这能修复每一个扩展频道(带盘符的路径根本不能作为原始 ESM specifier)。在 Linux/macOS 上,它修复含 URL 保留字符的路径——我在 Linux 上用
#复现了失败。(纯空格在 Node 22/Linux 上恰好仍能导入,但修复依然会正确编码它,这在更严格的环境和 Windows 上很重要。) - 顺序正确: 先
path.join(extPath, entry)(把清单里的相对 entry 拼到扩展根目录),再pathToFileURL(对整个绝对路径编码)。entry 保持相对于扩展根目录,符合设计。 - 范围最小且测试充分: 新逻辑是一个小巧、纯粹、可导出的函数,有直接单测覆盖;只改了 import 的 specifier。
|
@qwen-code /triage |
|
Thanks for the PR, @tt-a1i! Template deviations: the body uses "Summary" instead of "What this PR does" and is missing "Why it's needed", "Risk & Scope", "Linked Issues", and "中文说明". The substance is clear enough from context, but future PRs should follow the template for faster review. On direction: this is a clear, legitimate bug fix. Dynamic On approach: scope is minimal and well-targeted. Two files, +31/−3, one pure function extracted for testability. Nothing to cut — this is exactly the minimal change needed. Moving on to code review. 🔍 中文说明感谢贡献,@tt-a1i! 模板偏差:PR 使用了 "Summary" 而非 "What this PR does",缺少 "Why it's needed"、"Risk & Scope"、"Linked Issues" 和 "中文说明"。但从上下文可以看出实质内容,后续 PR 请遵循模板以加速审查。 方向:这是一个明确且合理的 bug 修复。动态 方案:范围极小且精准。两个文件,+31/−3,提取了一个纯函数以便测试。没有可裁剪的部分 —— 这就是所需的最小改动。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): the fix for The PR does exactly this. One observation (non-blocking): the unit test covers a space in the path ( Static Checks (worktree, PR branch)
Real-Scenario Test (tmux)Created a The 中文说明代码审查独立方案(阅读 diff 前):修复 PR 完全吻合。 一个观察(非阻断):单测覆盖了路径中的空格( 静态检查(worktree,PR 分支)
真实场景测试(tmux)创建了一个文件名含 原始路径中的 — Qwen Code · qwen3.7-max |
|
The PR's approach matches my independent proposal exactly: The tmux test confirms what the PR promises — raw paths with One minor wish: a second test case with Approving. ✅ 中文说明PR 的方案与我的独立方案完全一致:在 tmux 测试证实了 PR 的承诺 —— 含 一个小愿望:增加一个路径含 批准合并。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Summary
Test Plan
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.