fix(extension): accept uppercase URL schemes in Claude plugin sources - #5461
Conversation
resolvePluginSource compared a string plugin source against 'http://' and 'https://' case-sensitively, so a marketplace.json source such as 'HTTPS://github.com/owner/repo' fell through to local-path handling and failed with "Plugin source not found". Lowercase the source before the scheme check, matching QwenLM#5426 / QwenLM#5429 / QwenLM#5439.
|
The Lint job and all three Test jobs are red for the same reason — a TypeScript compile error, not an actual test failure. They all die inside Root cause: the new test mocks export async function downloadFromGitHubRelease(...): Promise<GitHubDownloadResult>
// GitHubDownloadResult = { tagName: string; type: 'git' | 'github-release' }
Fix — return a vi.mocked(downloadFromGitHubRelease).mockImplementation(
async (_meta, dir) => {
fs.mkdirSync(path.join(dir as string, '.claude-plugin'), { recursive: true });
fs.writeFileSync(
path.join(dir as string, '.claude-plugin', 'plugin.json'),
JSON.stringify({ name: 'p', version: '1.0.0' }),
'utf-8',
);
return { tagName: 'v1.0.0', type: 'github-release' }; // <-- add this
},
);The production change itself (the case-insensitive scheme check in 中文版Lint 和三个平台的 Test job 全红是同一个原因——一个 TypeScript 编译错误,并不是测试真的跑挂了。它们全部在 根因: 新增测试 mock 了 export async function downloadFromGitHubRelease(...): Promise<GitHubDownloadResult>
// GitHubDownloadResult = { tagName: string; type: 'git' | 'github-release' }
修法——在 mock 末尾返回一个 return { tagName: 'v1.0.0', type: 'github-release' }; // <-- 补这一行生产代码本身( |
The mockImplementation returned void, which tsc --build rejected (TS2345) even though vitest passed. Return a GitHubDownloadResult so the build is clean.
|
@qwen-code /triage |
|
Thanks for the PR @he-yufeng! Template: Most sections present (What, Why, Reviewer Test Plan, Risk), but a few template subsections are missing — "How to verify", "Evidence (Before & After)", "Tested on" table, "Linked Issues", and the Chinese translation. Not blocking on this since the PR content is clear enough, but worth filling in next time for faster review turnaround. Direction: Solid bugfix. The case-sensitive URL scheme check in Approach: Minimal and correct. Create Moving on to code review. 🔍 中文说明感谢 @he-yufeng 的贡献! 模板: 大部分章节齐全(What、Why、Reviewer Test Plan、Risk),但缺少部分模板子章节——"How to verify"、"Evidence (Before & After)"、"Tested on" 表格、"Linked Issues" 和中文翻译。不因此阻塞,但建议下次补全以加快审查速度。 方向: 合理的 bugfix。 方案: 最小改动且正确。创建 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: The fix for case-sensitive URL scheme matching in Diff assessment: The change is correct and minimal. Test ResultsBefore (reverted fix — main behavior)The new test fails, confirming the bug exists without the fix: After (with PR fix applied)All 41 tests pass, including the new uppercase URL scheme test: 中文说明代码审查独立方案: 修复 Diff 评估: 改动正确且最小化。 测试结果修复前: 新增测试失败,确认 bug 存在—— 修复后: 全部 41 个测试通过,包括新增的大写 URL scheme 测试。 — Qwen Code · qwen3.7-max |
Verdict: Approve ✅This is a clean, minimal bugfix that does exactly what it says. The case-sensitive URL scheme check was a real bug — an uppercase The test is well-crafted: it reproduces the exact failure mode (uppercase scheme → This follows the same pattern already established for MCP transport, extension install sources, and weixin CDN uploads. Consistent defensive coding. 中文说明结论:批准 ✅这是一个干净、最小化的 bugfix。大小写敏感的 URL scheme 检查是一个真实的 bug—— 测试设计良好:精确复现了失败场景(大写 scheme → 与 MCP transport、extension install、weixin CDN 的同类修复保持一致。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local verification — real tests (recommend merge)TL;DR: The fix is correct, minimal, and consistent with the sibling case-insensitivity fixes (#5426 / #5429 / #5439). The new test genuinely reproduces the bug, and there are no regressions. I built and ran the real Environment: Node 1) PR's own suite — greenIncludes the new 2) A/B revert-proof — the test really catches the bugReverting only the one-line fix (restoring the case-sensitive check) while keeping the new test:
The base-state error string matches the PR description verbatim ( 3) Branch/edge harness — 10 extra cases on the real code pathDrove the real
Discrimination (same harness, base vs PR):
→ Base: 6 failed / 4 passed; PR: 10 passed. (Run together with the PR's own file: 51 passed.) 4) Typecheck — validates the 3rd commit (vitest does not typecheck)
— that commit
Nit (non-blocking, in favor of the PR)The fix correctly uses Verdict: ✅ LGTM — safe to merge. 🇨🇳 中文版本(点击展开)✅ 本地真实测试验证(建议合并)结论: 修复正确、最小化,且与同类大小写不敏感修复(#5426 / #5429 / #5439)保持一致。新增测试能真实复现该 bug,且无任何回归。我在本地 tmux 会话中基于 PR HEAD( 环境: Node 1)PR 自带测试套件 — 通过其中包含新增用例 2)A/B 反证 — 证明该测试确实能抓住 bug仅回退这一行修复(恢复大小写敏感判断)、保留新增测试:
Base 状态下的报错字符串与 PR 描述完全一致( 3)分支/边界 harness — 在真实代码路径上补充 10 个用例直接驱动真实的
区分度(同一 harness,base vs PR):
→ Base:6 失败 / 4 通过;PR:10 通过。(与 PR 自带文件一起运行:51 通过。) 4)类型检查 — 验证第 3 个提交(vitest 不做类型检查)对 —— 这正是提交
小建议(不阻塞,且对 PR 有利)修复正确地使用了 结论:✅ LGTM —— 可以安全合并。 Verification artifacts: dedicated tmux session + worktree on PR head; full logs of all four layers retained locally. |
✅ Verification report — local real-world testing (routing fix correct; one important gap to consider)Verified locally on Linux (Node 22.22.2) with unit tests, a before/after regression check, a routing/downstream/sibling harness against the built bundle, and live What the bug was
Verification performed1. 2. Before/after (test is load-bearing). Reverting only the source to the old case-sensitive check makes the new test fail with the exact bug: Restoring the fix → passes. 3. Harness against the shipped
4. Live
A no longer produces C's local-path error → the routing fix works. But A and B then diverge (see below). 5. Static + CI.
|
What
Make the string plugin-source URL-scheme check in
resolvePluginSource(Claude plugin conversion) case-insensitive.Why
A string plugin
sourceinmarketplace.jsonwas compared againsthttp:///https://case-sensitively, so an uppercase scheme such asHTTPS://github.com/owner/repofell through to local-path handling and failed withPlugin source not found at .../HTTPS:/github.com/owner/repo.This is the same bug class already fixed for MCP transport detection (#5426), extension install sources (#5429), and weixin CDN uploads (#5439).
Reviewer Test Plan
npx vitest run packages/core/src/extension/claude-converter.test.tsconvertClaudePluginPackage — string URL source › treats an uppercase HTTPS:// source as a URL download, not a local pathfails before this change (throwsPlugin source not found) and passes after it.Risk
Low. One-line normalization — lowercase the source before the scheme check, matching the earlier fixes. Valid lowercase URLs and non-URL local paths are unaffected, and the rest of the
claude-convertersuite still passes.