Skip to content

fix(core): support public GitHub extensions with older Git #9680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yiliang114 merged 2 commits into from
Aug 21, 2026
Merged

fix(core): support public GitHub extensions with older Git #9680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2141b2a
fix(core): clarify Git requirement for public extensions
yiliang114 Aug 21, 2026
2bc440b
test(core): preserve secure Git version boundary
yiliang114 Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/users/extension/introduction.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ Only scoped packages (`@scope/package-name`) are supported to avoid ambiguity wi

#### From Git Repository

Public Git repository installs and update checks require Git 2.37 or newer. Qwen Code uses the `http.curloptResolve` setting introduced in Git 2.37 to pin public network connections to validated DNS results. If your distribution ships an older Git version, upgrade Git or install a local/archive release instead.

```bash
qwen extensions install https://github.com/github/github-mcp-server
```
Expand Down
49 changes: 49 additions & 0 deletions packages/core/src/extension/github.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,55 @@ describe('git extension helpers', () => {
);
});

it('explains how to install public extensions when Git is too old for DNS pinning', async () => {
mockGit.version.mockResolvedValue({ major: 2, minor: 34, patch: 1 });
Comment on lines +322 to +323

@qwen-code-ci-bot qwen-code-ci-bot Aug 21, 2026

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] Neither new test exercises the undefined-component branch of .filter((component) => component !== undefined) — both supply major, minor, and patch. A mutation probe confirmed that deleting the filter keeps both new tests green (with inputs {2,34,1} and {2,37,0} the join output is identical either way). The discriminating input is a version result without a patch component, which the filter itself anticipates: without the filter the message degrades to found Git 2.34.. (double dot — join renders undefined as the empty string, observed in the probe), and no test fails. Add a case like:

it('renders the detected version without a patch component', async () => {
  mockGit.version.mockResolvedValue({ major: 2, minor: 34 });

  await expect(
    cloneFromGit(
      {
        source: 'https://github.com/owner/repo.git',
        type: 'git',
        networkPolicy: 'public',
      },
      '/dest',
    ),
  ).rejects.toThrow('found Git 2.34. Upgrade Git');
  expect(mockGit.clone).not.toHaveBeenCalled();
});

The assertion found Git 2.34. Upgrade Git fails against the double-dot output, so it pins the filter's behavior.

中文说明

两个新测试都没有覆盖 .filter((component) => component !== undefined) 的 undefined 分量分支——它们都提供了 major、minor 和 patch。变异探针确认：删除该 filter 后两个新测试仍然通过（对于输入 {2,34,1} 和 {2,37,0}，join 的输出在两种情况下完全相同）。判定性输入是不含 patch 分量的版本结果——这正是 filter 本身所预期的场景：若没有 filter，消息会退化为 found Git 2.34..（双点——join 把 undefined 渲染为空字符串，探针已观察到），且没有任何测试失败。建议补充如上代码块所示的用例；断言 found Git 2.34. Upgrade Git 在出现双点输出时会失败，从而钉住 filter 的行为。

— qwen3.8-max via Qwen Code /review (v0.21.15)


await expect(
cloneFromGit(
{
source: 'https://github.com/owner/repo.git',
type: 'git',
networkPolicy: 'public',
},
'/dest',
),
).rejects.toThrow(
'Public extension Git installs require Git 2.37 or newer for secure DNS pinning; found Git 2.34.1. Upgrade Git, or install the extension from a local path or archive instead.',
);
expect(mockGit.clone).not.toHaveBeenCalled();
});

it('accepts Git 2.37 while preserving public network pinning', async () => {
mockGit.version.mockResolvedValue({ major: 2, minor: 37, patch: 0 });
vi.spyOn(dns, 'lookup').mockResolvedValue([
{ address: '8.8.8.8', family: 4 },
] as never);
const source = 'https://github.com/owner/repo.git';
mockGit.getRemotes.mockResolvedValue([
{ name: 'origin', refs: { fetch: source } },
]);

await cloneFromGit(
{ source, type: 'git', networkPolicy: 'public' },
'/dest',
);

expect(simpleGit).toHaveBeenLastCalledWith('/dest', {
config: [
'http.curloptResolve=github.com:443:8.8.8.8',
'http.followRedirects=false',
'http.proxy=',
'protocol.allow=never',
'protocol.https.allow=always',
],
unsafe: {
allowUnsafeConfigPaths: true,
allowUnsafeProtocolOverride: true,
},
});
expect(mockGit.clone).toHaveBeenCalled();
});

it('passes explicit credentials through scoped Git config without changing the URL', async () => {
vi.spyOn(dns, 'lookup').mockResolvedValue([
{ address: '8.8.8.8', family: 4 },
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/extension/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,12 @@ async function assertPinnedGitSupported(): Promise<void> {
(version.major === MINIMUM_PINNED_GIT_VERSION.major &&
version.minor < MINIMUM_PINNED_GIT_VERSION.minor)
) {
throw new Error('Public extension Git installs require Git 2.37 or newer.');
const detectedVersion = [version.major, version.minor, version.patch]
.filter((component) => component !== undefined)
.join('.');
Comment on lines +147 to +149

@qwen-code-ci-bot qwen-code-ci-bot Aug 21, 2026

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] When Git is not installed at all, simple-git's version() resolves (does not throw) with {major: 0, minor: 0, patch: 0, installed: false}, and the new code ignores the installed flag — so the message reports found Git 0.0.0. Verified against the installed simple-git 3.36.0 with git removed from PATH and end-to-end through this PR's own cloneFromGit: the user sees "…found Git 0.0.0. Upgrade Git, or install the extension from a local path or archive instead." The diagnostic contradicts reality (nothing was found) and sends them hunting for a phantom 0.0.0 install to upgrade; the rejection itself is correct. Branch on the flag simple-git already provides:

Suggested change
const detectedVersion = [version.major, version.minor, version.patch]
.filter((component) => component !== undefined)
.join('.');
if (!version.installed) {
throw new Error(
'Public extension Git installs require Git 2.37 or newer for secure DNS pinning; no Git installation was found. Install Git 2.37 or newer, or install the extension from a local path or archive instead.',
);
}
const detectedVersion = [version.major, version.minor, version.patch]
.filter((component) => component !== undefined)
.join('.');

(plus a test mocking { major: 0, minor: 0, patch: 0, installed: false })

中文说明

当完全没有安装 Git 时，simple-git 的 version() 会 resolve（而不是抛错）并返回 {major: 0, minor: 0, patch: 0, installed: false}，而新代码忽略了 installed 标志——因此消息显示 found Git 0.0.0。已在安装的 simple-git 3.36.0 上（将 git 从 PATH 移除）以及通过本 PR 自身的 cloneFromGit 端到端验证：用户会看到 "…found Git 0.0.0. Upgrade Git, or install the extension from a local path or archive instead."。该诊断与现实矛盾（根本没有找到 Git），会让人去找一个并不存在的 0.0.0 安装来"升级"；拒绝行为本身是正确的。建议利用 simple-git 已提供的标志进行分支（如上 suggestion），并补充一个 mock { major: 0, minor: 0, patch: 0, installed: false } 的测试。

— qwen3.8-max via Qwen Code /review (v0.21.15)

throw new Error(
`Public extension Git installs require Git 2.37 or newer for secure DNS pinning; found Git ${detectedVersion}. Upgrade Git, or install the extension from a local path or archive instead.`,

@qwen-code-ci-bot qwen-code-ci-bot Aug 21, 2026

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 minimum version is hardcoded as the literal "2.37" in this message, duplicating MINIMUM_PINNED_GIT_VERSION (github.ts:47); the new docs sentence and the new test expectation bake the same literal, so four sites must be kept in sync by hand. A future maintainer who bumps the constant (say, to 2.40) and updates only the comparison leaves a user on Git 2.38 with the self-contradictory error "require Git 2.37 or newer ...; found Git 2.38.1" while the docs still say 2.37 suffices — and the test keeps passing on the stale string. Derive the requirement from the constant (verified in a scratch tree: both new tests stay green, rendered string identical):

Suggested change
`Public extension Git installs require Git 2.37 or newer for secure DNS pinning; found Git ${detectedVersion}. Upgrade Git, or install the extension from a local path or archive instead.`,
`Public extension Git installs require Git ${MINIMUM_PINNED_GIT_VERSION.major}.${MINIMUM_PINNED_GIT_VERSION.minor} or newer for secure DNS pinning; found Git ${detectedVersion}. Upgrade Git, or install the extension from a local path or archive instead.`,
中文说明

最低版本在这条消息中被硬编码为字面量 "2.37"，与 MINIMUM_PINNED_GIT_VERSION（github.ts:47）重复；新增的文档句子和新测试的期望值也写死了同一个字面量，因此四处必须手工保持同步。若未来维护者提高该常量（例如到 2.40）而只更新比较逻辑，使用 Git 2.38 的用户会收到自相矛盾的错误 "require Git 2.37 or newer ...; found Git 2.38.1"，而文档仍说 2.37 足够——测试也会因陈旧的字符串而继续通过。建议从常量派生版本要求（已在 scratch tree 验证：两个新测试仍然通过，渲染的字符串完全相同）。

— qwen3.8-max via Qwen Code /review (v0.21.15)

);
Comment on lines +150 to +152

@qwen-code-ci-bot qwen-code-ci-bot Aug 21, 2026

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 improved message never reaches the second of its two consumers. On the update-check path, checkForExtensionUpdate calls assertPinnedGitSupported() (github.ts:482), but its catch-all (github.ts:602-609) logs the error to debugLogger.error only and returns a bare ExtensionUpdateState.ERROR, so the update-check coverage this PR claims — and the new docs sentence "installs and update checks require Git 2.37 or newer" — is never surfaced at runtime. A user on Git < 2.37 who triggers an update check for a networkPolicy: 'public' git extension sees only Failed to check "<name>" for updates. in the TUI (the headless extensions update command even prints "already up to date" for the ERROR state) — no Git version, cause, or alternative. A probe driving the real checkForExtensionUpdate with a Git 2.34.1 extension confirmed the error does not propagate (F1-THREW: false F1-RESULT: "error"). One possible shape — mirror the ExtensionCredentialUnavailableError treatment:

class ExtensionGitTooOldError extends Error {}

// assertPinnedGitSupported: throw new ExtensionGitTooOldError(...)
// checkForExtensionUpdate's catch:
if (
  error instanceof ExtensionCredentialUnavailableError ||
  error instanceof ExtensionGitTooOldError
) {
  throw error;
}
// note: checkAllExtensionsForUpdates has its own .catch(() => ERROR)
// (extensionManager.ts:3015-3019) that would need the same rethrow
中文说明

改进后的消息没有到达它的第二个使用者。在更新检查路径上，checkForExtensionUpdate 调用了 assertPinnedGitSupported()（github.ts:482），但其 catch-all（github.ts:602-609）只把错误记录到 debugLogger.error 并返回裸的 ExtensionUpdateState.ERROR，因此 PR 声称的更新检查覆盖——以及新增文档句子"公共 Git 仓库的安装和更新检查需要 Git 2.37 或更高版本"——在运行时并不会真正呈现。使用 Git < 2.37 的用户触发 networkPolicy: 'public' git 扩展的更新检查时，TUI 只会显示 Failed to check "<name>" for updates.（headless 的 extensions update 命令甚至会对 ERROR 状态打印 "already up to date"）——没有 Git 版本、原因或替代方案。用 Git 2.34.1 扩展驱动真实 checkForExtensionUpdate 的探针确认错误没有向上传播（F1-THREW: false F1-RESULT: "error"）。建议参照 ExtensionCredentialUnavailableError 的处理方式：从 assertPinnedGitSupported 抛出专用错误类，并将其加入 checkForExtensionUpdate catch 中的重新抛出集合（注意 checkAllExtensionsForUpdates 有自己的 .catch(() => ERROR)，extensionManager.ts:3015-3019，也需要同样的重新抛出）。

— qwen3.8-max via Qwen Code /review (v0.21.15)

}
}

Expand Down
Loading