fix(extensions): handle uppercase npm registry schemes

[tt-a1i]

What this PR does

Makes npm extension registry requests choose the HTTP client from the parsed URL protocol instead of checking for a lowercase https:// prefix. This keeps uppercase HTTPS registry and tarball URLs on the HTTPS client.

Why it is needed

URL schemes are case-insensitive, but the npm extension path used a case-sensitive string prefix check. A registry URL like HTTPS://registry.npmjs.org could be misrouted to http.get, and the same issue applied when downloading tarballs from uppercase HTTPS tarball URLs.

Reviewer Test Plan

How to verify

Run npx vitest run packages/core/src/extension/npm.test.ts, npm run typecheck --workspace=packages/core, npm run lint --workspace=packages/core, npm run build --workspace=packages/core, and git diff --check. The new tests assert uppercase HTTPS registry metadata and tarball URLs use https.get and never call http.get.

Evidence (Before & After)

N/A

Tested on

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

Environment (optional)

Local npm workspace on macOS.

Risk & Scope

• Main risk or tradeoff: Low. This only changes client selection for npm registry URLs and now rejects unsupported URL protocols instead of accidentally sending them through the HTTP client.
• Not validated / out of scope: Manual install against every possible registry service; Windows and Linux local runs.
• Breaking changes / migration notes: None.

Linked Issues

Fixes #5436

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

中文说明

这个 PR 做了什么

让 npm extension registry 请求根据解析后的 URL protocol 选择 HTTP client，而不是检查小写 https:// 前缀。这样 uppercase HTTPS registry 和 tarball URL 都会继续走 HTTPS client。

为什么需要

URL scheme 本身是大小写不敏感的，但 npm extension 路径之前用了大小写敏感的字符串前缀判断。像 HTTPS://registry.npmjs.org 这样的 registry URL 可能会被错误转到 http.get，下载 uppercase HTTPS tarball URL 时也有同样问题。

Reviewer Test Plan

如何验证

运行 npx vitest run packages/core/src/extension/npm.test.ts、npm run typecheck --workspace=packages/core、npm run lint --workspace=packages/core、npm run build --workspace=packages/core 和 git diff --check。新增测试会断言 uppercase HTTPS registry metadata 和 tarball URL 都使用 https.get，不会调用 http.get。

Before & After 证据

N/A

Tested on

macOS 已本地验证；Windows 和 Linux 未本地验证，交给 CI 覆盖。

Environment

macOS 本地 npm workspace。

Risk & Scope

• 主要风险或取舍：低。改动只影响 npm registry URL 的 client 选择，并且现在会拒绝 unsupported URL protocol，而不是意外交给 HTTP client。
• 未验证 / 不在范围内：没有手动覆盖每一种 registry 服务；Windows 和 Linux 未本地跑。
• Breaking changes / migration notes：无。

Linked Issues

Fixes #5436

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

[wenshao]

✅ Local verification — recommend merge

I built and tested this PR locally as a merge reference. Verdict: verified, safe to merge. The fix is correct, minimal, well-scoped, and the two new tests genuinely guard it (proven by a mutation test).

Environment: dedicated git worktree at PR head de3e22f5 on top of origin/main 61dcf865 (clean, no rebase needed) · macOS (darwin-arm64) · Node v22.22.2 · Vitest 3.2.4.

Root cause confirmed

URL schemes are case-insensitive, and the WHATWG URL parser already normalizes the scheme to lowercase — but the old literal prefix check does not:

input	new URL(input).protocol (fix)	input.startsWith('https://') (old)
HTTPS://registry.example.com	"https:" → https client ✅	false → http client ❌ (the bug)
https://…	"https:" ✅	true ✅
HTTP://…	"http:" ✅	false → http ✅

So an uppercase HTTPS:// registry/tarball URL was misrouted to http.get. Confirmed.

Verification results

1. PR test suite (fixed code) — all green

npx vitest run packages/core/src/extension/npm.test.ts
✓ src/extension/npm.test.ts (26 tests) — Tests 26 passed (26)

2. Mutation test (the decisive proof) — the new tests are non-vacuous
Reverted only npm.ts to base (git checkout de3e22f5^ -- npm.ts, git diff --stat empty), kept the PR's test file, re-ran:

Tests  2 failed | 24 passed (26)

× downloadFromNpmRegistry > uses the HTTPS client for uppercase HTTPS tarball URLs
  → promise rejected "Error: wrong client" instead of resolving
× checkNpmUpdate > uses the HTTPS client for uppercase HTTPS registry URLs
  → expected 'error' to be 'up to date'

Exactly the two new tests fail (each because base routes uppercase HTTPS to the http.get mock that throws wrong client); the other 24 stay green. This proves the tests actually exercise the fix and that the change is scoped to uppercase-HTTPS routing with no collateral impact.

3. Reviewer Test Plan gates — all pass

gate	result
npm run typecheck --workspace=packages/core	✅ exit 0
npm run lint --workspace=packages/core	✅ exit 0
npm run build --workspace=packages/core	✅ exit 0 (built dist contains the fix)
git diff --check	✅ exit 0 (no whitespace errors)

Reverse audit

• Complete for the npm path. fetchNpmJson and downloadNpmFile are the only two client-selection sites in npm.ts; both now route through clientForUrl.
• No new failure surface from new URL() throwing. Every URL that reaches clientForUrl is already parsed by a prior new URL() on the same path — registryUrl via getNpmAuthToken (and metadataUrl derives from it), tarballUrl via the tarball-host check, and redirect location via the redirect-host check. So the throw only turns already-doomed inputs into a clearer error; the genuinely new behavior is the intended hardening: a parseable-but-unsupported scheme (ftp:, file:, …) is now rejected explicitly instead of silently falling through to the HTTP client. checkNpmUpdate additionally wraps everything in try/catch, so it degrades to ERROR gracefully.
• Nit (non-blocking): .toLowerCase() on new URL(url).protocol is redundant since the URL parser already lowercases the scheme — harmless and defensive, no change needed.
• Out of scope (optional follow-up, not a defect of this PR): the same case-sensitive startsWith('http(s)://') pattern still exists in unrelated paths — URL type-detection in marketplace.ts:286, sourceRegistry.ts:131, claude-converter.ts:1006, qwenOAuth2.ts:724, and a client-selection site in channels/weixin/src/api.ts:346. These are outside issue Npm extension registry fetch misroutes uppercase HTTPS URLs #5436's scope (the npm registry fetch path) and I did not verify them; flagging only for completeness.

Conclusion

Correct root-cause fix, minimal diff, non-vacuous regression coverage, clean typecheck/lint/build. LGTM — recommend merge.

中文版

✅ 本地验证 —— 建议合并

我在本地构建并测试了该 PR，作为合并参考。结论：验证通过，可安全合并。 修复正确、改动最小、范围清晰，且两个新增测试确实能守护该修复（已用变异测试证明）。

环境： 在 origin/main 61dcf865 之上、PR head de3e22f5 的独立 git worktree（干净，无需 rebase）· macOS（darwin-arm64）· Node v22.22.2 · Vitest 3.2.4。

根因确认

URL scheme 大小写不敏感，WHATWG URL 解析器本身就会把 scheme 规范化为小写——但旧的字面前缀判断不会：

输入	new URL(input).protocol（修复）	input.startsWith('https://')（旧）
HTTPS://registry.example.com	"https:" → https client ✅	false → http client ❌（即 bug）
https://…	"https:" ✅	true ✅
HTTP://…	"http:" ✅	false → http ✅

所以大写的 HTTPS:// registry/tarball URL 会被误路由到 http.get。已确认。

验证结果

1. PR 测试套件（修复版）—— 全绿

npx vitest run packages/core/src/extension/npm.test.ts
✓ src/extension/npm.test.ts (26 tests) —— Tests 26 passed (26)

2. 变异测试（决定性证据）—— 新测试非空过
仅把 npm.ts 还原到 base（git checkout de3e22f5^ -- npm.ts，git diff --stat 为空），保留 PR 的测试文件，重跑：

Tests  2 failed | 24 passed (26)

× downloadFromNpmRegistry > uses the HTTPS client for uppercase HTTPS tarball URLs
  → promise rejected "Error: wrong client" instead of resolving
× checkNpmUpdate > uses the HTTPS client for uppercase HTTPS registry URLs
  → expected 'error' to be 'up to date'

恰好是这两个新测试失败（因为 base 把大写 HTTPS 路由到了会抛 wrong client 的 http.get mock）；其余 24 个保持绿色。这证明测试确实覆盖了该修复，并且改动精确作用于大写 HTTPS 路由、没有连带影响。

3. Reviewer Test Plan 各项 gate —— 全部通过

gate	结果
npm run typecheck --workspace=packages/core	✅ exit 0
npm run lint --workspace=packages/core	✅ exit 0
npm run build --workspace=packages/core	✅ exit 0（产物 dist 含修复）
git diff --check	✅ exit 0（无空白错误）

反向审计

• npm 路径上修复完整。 fetchNpmJson 与 downloadNpmFile 是 npm.ts 中仅有的两处 client 选择点，现都经由 clientForUrl。
• new URL() 抛错未引入新的失败面。 每个到达 clientForUrl 的 URL，在同一路径里都已被先前的 new URL() 解析过——registryUrl 经 getNpmAuthToken（metadataUrl 由其派生）、tarballUrl 经 tarball-host 检查、重定向 location 经 redirect-host 检查。因此该 throw 只是把"本来就会失败"的输入变成更清晰的错误；真正新增的行为是有意的加固：可解析但不支持的 scheme（ftp:、file: 等）现在被明确拒绝，而不再静默落到 HTTP client。checkNpmUpdate 另有 try/catch 包裹，会优雅降级为 ERROR。
• 小提示（不阻塞）： 对 new URL(url).protocol 再 .toLowerCase() 是冗余的（URL 解析器已小写化 scheme）——无害且防御性，无需改动。
• 范围外（可选后续，非本 PR 缺陷）： 同样的大小写敏感 startsWith('http(s)://') 模式仍存在于其它无关路径——URL 类型探测 见 marketplace.ts:286、sourceRegistry.ts:131、claude-converter.ts:1006、qwenOAuth2.ts:724，以及 channels/weixin/src/api.ts:346 的一处 client 选择。它们不属于 issue Npm extension registry fetch misroutes uppercase HTTPS URLs #5436 的范围（npm registry 拉取路径），我也未对其验证；仅出于完整性提示。

结论

正确的根因修复、最小 diff、非空过的回归覆盖、typecheck/lint/build 全清。LGTM —— 建议合并。

[wenshao]

No blocking issues — clean fix, and actually a small security improvement. The old url.startsWith('https://') ? https : http was case-sensitive, so an uppercase HTTPS:// registry/tarball URL fell through to the http client — sending the Bearer auth token over plaintext. The new clientForUrl parses the protocol and routes https: over TLS (and throws on unsupported schemes instead of silently downgrading). The 2 new tests genuinely guard it (they fail on main), and every call site already validates the URL via new URL() upstream, so the new parse adds no new failure mode. 26/26 tests pass locally.

Optional, non-blocking nit: the new clientForUrl was inserted between the /** Fetch JSON from a URL… */ JSDoc and fetchNpmJson, so that comment now documents the wrong function (and fetchNpmJson lost its doc). Trivial to move.

⚠️ Downgraded from Approve to Comment: CI still running.

中文

无阻断问题 —— 干净的修复,而且其实是个小的安全改进。旧的 url.startsWith('https://') ? https : http 大小写敏感,所以大写的 HTTPS:// registry/tarball URL 会落到 http 客户端 —— 把 Bearer 鉴权 token 走明文发出去。新的 clientForUrl 解析协议并把 https: 走 TLS(对不支持的 scheme 直接抛错,而不是静默降级到 http)。两个新测试确实守护了该行为(在 main 上会失败),而且每个调用点上游都已用 new URL() 校验过 URL,所以这次新增的解析不引入新的失败路径。本地 26/26 测试通过。

可选、非阻塞的小问题:新的 clientForUrl 被插在了 /** Fetch JSON from a URL… */ 这段 JSDoc 与 fetchNpmJson 之间,导致该注释现在文档化了错误的函数(而 fetchNpmJson 丢了自己的注释)。挪一下即可。

⚠️ 由 Approve 降级为 Comment:CI 仍在运行。

— claude-opus-4-8 via Claude Code /qreview

[wenshao]

[Suggestion] The JSDoc comment /** Fetch JSON from a URL, handling both https and http. */ at line 183 now sits directly above clientForUrl, but it describes fetchNpmJson. clientForUrl returns a module reference, not JSON.

Suggested change

	function clientForUrl(url: string): typeof https | typeof http {
	/**
	* Return the http or https client module for the given URL's protocol.
	*/
	function clientForUrl(url: string): typeof https | typeof http {

…and restore the original JSDoc above fetchNpmJson:

Suggested change

	function clientForUrl(url: string): typeof https | typeof http {
	/**
	* Fetch JSON from a URL, handling both https and http.
	*/
	function fetchNpmJson<T>(url: string, authToken?: string): Promise<T> {

— qwen3.7-max via Qwen Code /review

[wenshao]

[Suggestion] The new clientForUrl function has three branches (https:, http:, unsupported-protocol throw), but the two new tests only exercise the https: path via uppercase HTTPS:// URLs. The http: branch (e.g., an HTTP:// registry URL) and the unsupported-protocol error path are untested.

Consider adding a test that uses an HTTP:// registry URL and asserts https.get is NOT called, plus a test that passes a ftp:// URL and asserts the call rejects with "Unsupported npm registry URL protocol". This would lock in the fix for both sides of the case-insensitivity bug.

— qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

On direction: clear bug fix — url.startsWith('https://') is case-sensitive and misroutes uppercase HTTPS URLs to the HTTP client. URL schemes are case-insensitive per RFC 3986, so this is a real correctness issue, even though uppercase schemes are rare in practice. Linked issue #5436 describes it well. No direction concerns.

On approach: the fix is minimal and focused — a small clientForUrl() helper that parses the URL properly and replaces the two startsWith checks. 2 files, +116/-2 (most of the additions are test setup). No scope creep, no drive-by refactors. The added bonus of rejecting unsupported protocols (instead of silently falling through to http) is a nice hardening touch.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

方向：明确的 bug 修复——url.startsWith('https://') 大小写敏感，会把大写 HTTPS URL 错误路由到 HTTP client。URL scheme 按 RFC 3986 是大小写不敏感的，虽然实际中大写 scheme 很少见，但这确实是个正确性问题。关联 issue #5436 描述清楚，方向没有问题。

方案：改动精简聚焦——引入 clientForUrl() 辅助函数做 URL 解析，替换两处 startsWith 检查。2 个文件，+116/-2（大部分是测试 setup）。没有 scope creep，没有顺手重构。额外拒绝不支持的 protocol（而不是默默走 http）也是好的加固。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal: parse the URL with new URL(url).protocol (returns lowercase), extract a small helper since there are two call sites, and reject unsupported protocols instead of silently defaulting to HTTP. The PR does exactly this — approach matches perfectly.

The clientForUrl() helper is clean and correct. It uses new URL(url).protocol.toLowerCase() for case-insensitive detection, returns the right client, and throws for unsupported protocols. Both call sites in fetchNpmJson and downloadNpmFile are updated. No code review concerns — no correctness bugs, no security issues, no convention violations.

The two new tests are well-structured: they mock http.get to throw if called (proving it's never used), and verify https.get handles both uppercase registry metadata URLs and uppercase tarball URLs.

Typecheck ✅, lint ✅, build ✅, 26/26 unit tests ✅ (24 existing + 2 new).

Before/After Testing

Test script imports the compiled npm.js, inspects the client-selection logic, then calls checkNpmUpdate with HTTPS://registry.npmjs.org (uppercase scheme).

Before (main branch — bug present)

=== Compiled npm.js client-selection analysis ===
Has startsWith('https://') check: true
Has clientForUrl() helper:        false

BUG PRESENT: case-sensitive startsWith check routes
uppercase HTTPS URLs to http.get instead of https.get

=== Runtime test: uppercase HTTPS registry URL ===
registryUrl: HTTPS://registry.npmjs.org
checkNpmUpdate result: error

On main, the uppercase HTTPS URL is routed to http.get (port 80), which fails to reach npmjs.org → returns error.

After (this PR)

=== Compiled npm.js client-selection analysis ===
Has startsWith('https://') check: false
Has clientForUrl() helper:        true

FIXED: clientForUrl() uses URL parsing for case-insensitive
protocol detection — uppercase HTTPS routes to https.get

=== Runtime test: uppercase HTTPS registry URL ===
registryUrl: HTTPS://registry.npmjs.org
checkNpmUpdate result: update available

With the fix, the uppercase HTTPS URL is correctly routed to https.get (port 443), successfully queries npmjs.org, and returns update available.

Bug confirmed and fix verified. ✅

中文说明

代码审查

独立方案：用 new URL(url).protocol（返回小写）解析 URL，提取辅助函数（两处调用），拒绝不支持的协议而不是默认走 HTTP。PR 完全吻合这个思路。

clientForUrl() 简洁正确，用 new URL(url).protocol.toLowerCase() 做大小写不敏感检测，返回正确 client，不支持的协议直接抛错。fetchNpmJson 和 downloadNpmFile 两处都已更新。没有正确性 bug、安全问题或规范违反。

两个新测试结构良好：mock http.get 被调用则抛错（证明不会被使用），验证 https.get 正确处理大写 registry metadata URL 和 tarball URL。

Typecheck ✅，lint ✅，build ✅，26/26 单测 ✅（24 现有 + 2 新增）。

Before/After 测试

测试脚本导入编译后的 npm.js，检查 client 选择逻辑，然后用 HTTPS://registry.npmjs.org（大写 scheme）调用 checkNpmUpdate。

main 分支上，大写 HTTPS URL 被路由到 http.get（80 端口），无法连接 npmjs.org → 返回 error。

PR 修复后，大写 HTTPS URL 正确路由到 https.get（443 端口），成功查询 npmjs.org → 返回 update available。

Bug 已确认，修复已验证。✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Clean fix, ships it.

The bug is real — url.startsWith('https://') is case-sensitive and misroutes uppercase HTTPS URLs to http.get. The fix is exactly right: a small clientForUrl() helper using new URL(url).protocol for case-insensitive detection, replacing both call sites. No scope creep, no over-engineering, no drive-by refactors. The bonus hardening (rejecting unsupported protocols instead of silently defaulting to HTTP) is a nice touch.

Before/after testing is conclusive: on main, uppercase HTTPS://registry.npmjs.org returns error (routed to port 80). With the fix, it returns update available (routed to port 443). Unit tests go from 24 → 26, all passing. Typecheck, lint, build all clean.

Approving. ✅

中文说明

干净的修复，可以合并。

Bug 确实存在——url.startsWith('https://') 大小写敏感，大写 HTTPS URL 会被路由到 http.get。修复完全正确：用 new URL(url).protocol 做大小写不敏感检测的 clientForUrl() 辅助函数，替换两处调用。没有 scope creep，没有过度设计，没有顺手重构。额外加固（拒绝不支持的协议而不是默默走 HTTP）也很好。

Before/after 测试结论明确：main 分支上大写 HTTPS://registry.npmjs.org 返回 error（走了 80 端口）；修复后返回 update available（走了 443 端口）。单测 24 → 26，全部通过。Typecheck、lint、build 全部干净。

批准。✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

🅷 Hold for consolidation — not a code objection (approval stands).

The fix itself is correct and even a small security improvement (uppercase HTTPS:// registry/tarball URLs no longer fall through to http.get), so the approval stays. This is purely a "please don't merge standalone yet" note.

#5437 (npm.ts) and #5435 (marketplace.ts) each introduce their own "pick the http/https client from the parsed URL protocol" helper. Landing them independently leaves two near-duplicate helpers — the same duplication we just ended up with across #5491 / #5496.

Could we please either:

1. extract a single shared helper (e.g. clientForUrl(url)) used by both call sites and land it once; or
2. fold the npm.ts change into fix(extensions): accept uppercase marketplace source schemes #5435 and close this PR.

Either way, let's land the shared util once rather than two copies. Thanks!

中文说明

🅷 暂缓合并 —— 不是代码问题,approve 保留。

这个修复本身是对的,还是个小安全改进(大写 HTTPS:// 的 registry/tarball URL 不再漏判走 http.get),所以 approve 我保留。这条只是"先别独立合"的提醒。

#5437(npm.ts)和 #5435(marketplace.ts)各自实现了一个"根据解析出的 URL 协议选择 http/https client"的 helper。如果分别独立合并,仓库里会留下两个几乎一样的 helper(跟刚才 #5491 / #5496 的重复 helper 是同一类问题)。

能否二选一:

1. 抽一个共用 helper(比如 clientForUrl(url)),两个调用点都用,只落地一次;或
2. 把 npm.ts 的改动并进 fix(extensions): accept uppercase marketplace source schemes #5435,关掉本 PR。

无论哪种,让这个共用 util 只进仓库一次,而不是两份。谢谢!