Don't always fall back on a git clone when installing extensions (#11229)

This commit is contained in:
Jacob MacDonald
2025-10-15 14:29:16 -07:00
committed by GitHub
parent 1fc3fc0a22
commit 8c1656bf56
4 changed files with 314 additions and 46 deletions

View File

@@ -29,11 +29,11 @@ import {
ExtensionDisableEvent,
ExtensionEnableEvent,
} from '@google/gemini-cli-core';
import { execSync } from 'node:child_process';
import { SettingScope } from './settings.js';
import { isWorkspaceTrusted } from './trustedFolders.js';
import { createExtension } from '../test-utils/createExtension.js';
import { ExtensionEnablementManager } from './extensions/extensionEnablement.js';
import { join } from 'node:path';
const mockGit = {
clone: vi.fn(),
@@ -47,6 +47,17 @@ const mockGit = {
path: vi.fn(),
};
const mockDownloadFromGithubRelease = vi.hoisted(() => vi.fn());
vi.mock('./extensions/github.js', async (importOriginal) => {
const original =
await importOriginal<typeof import('./extensions/github.js')>();
return {
...original,
downloadFromGitHubRelease: mockDownloadFromGithubRelease,
};
});
vi.mock('simple-git', () => ({
simpleGit: vi.fn((path: string) => {
mockGit.path.mockReturnValue(path);
@@ -123,8 +134,6 @@ describe('extension tests', () => {
source: undefined,
});
vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir);
vi.mocked(execSync).mockClear();
Object.values(mockGit).forEach((fn) => fn.mockReset());
});
afterEach(() => {
@@ -813,6 +822,11 @@ describe('extension tests', () => {
);
});
mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]);
mockDownloadFromGithubRelease.mockResolvedValue({
success: false,
failureReason: 'no release data',
type: 'github-release',
});
await installOrUpdateExtension(
{ source: gitUrl, type: 'git' },
@@ -1087,6 +1101,173 @@ This extension will run the following MCP servers:
),
).rejects.toThrow('Invalid extension name: "bad_name"');
});
describe('installing from github', () => {
const gitUrl = 'https://github.com/google/gemini-test-extension.git';
const extensionName = 'gemini-test-extension';
beforeEach(() => {
// Mock the git clone behavior for github installs that fallback to it.
mockGit.clone.mockImplementation(async (_, destination) => {
fs.mkdirSync(path.join(mockGit.path(), destination), {
recursive: true,
});
fs.writeFileSync(
path.join(mockGit.path(), destination, EXTENSIONS_CONFIG_FILENAME),
JSON.stringify({ name: extensionName, version: '1.0.0' }),
);
});
mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]);
});
afterEach(() => {
vi.restoreAllMocks();
});
it('should install from a github release successfully', async () => {
const targetExtDir = path.join(userExtensionsDir, extensionName);
mockDownloadFromGithubRelease.mockResolvedValue({
success: true,
tagName: 'v1.0.0',
type: 'github-release',
});
const tempDir = path.join(tempHomeDir, 'temp-ext');
fs.mkdirSync(tempDir, { recursive: true });
createExtension({
extensionsDir: tempDir,
name: extensionName,
version: '1.0.0',
});
vi.spyOn(ExtensionStorage, 'createTmpDir').mockResolvedValue(
join(tempDir, extensionName),
);
await installOrUpdateExtension(
{ source: gitUrl, type: 'github-release' },
async () => true,
);
expect(fs.existsSync(targetExtDir)).toBe(true);
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
expect(fs.existsSync(metadataPath)).toBe(true);
const metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf-8'));
expect(metadata).toEqual({
source: gitUrl,
type: 'github-release',
releaseTag: 'v1.0.0',
});
});
it('should fallback to git clone if github release download fails and user consents', async () => {
mockDownloadFromGithubRelease.mockResolvedValue({
success: false,
failureReason: 'failed to download asset',
errorMessage: 'download failed',
type: 'github-release',
});
const requestConsent = vi.fn().mockResolvedValue(true);
await installOrUpdateExtension(
{ source: gitUrl, type: 'github-release' }, // Use github-release to force consent
requestConsent,
);
// It gets called once to ask for a git clone, and once to consent to
// the actual extension features.
expect(requestConsent).toHaveBeenCalledTimes(2);
expect(requestConsent).toHaveBeenCalledWith(
expect.stringContaining(
'Would you like to attempt to install via "git clone" instead?',
),
);
expect(mockGit.clone).toHaveBeenCalled();
const metadataPath = path.join(
userExtensionsDir,
extensionName,
INSTALL_METADATA_FILENAME,
);
const metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf-8'));
expect(metadata.type).toBe('git');
});
it('should throw an error if github release download fails and user denies consent', async () => {
mockDownloadFromGithubRelease.mockResolvedValue({
success: false,
errorMessage: 'download failed',
type: 'github-release',
});
const requestConsent = vi.fn().mockResolvedValue(false);
await expect(
installOrUpdateExtension(
{ source: gitUrl, type: 'github-release' },
requestConsent,
),
).rejects.toThrow(
`Failed to install extension ${gitUrl}: download failed`,
);
expect(requestConsent).toHaveBeenCalledExactlyOnceWith(
expect.stringContaining(
'Would you like to attempt to install via "git clone" instead?',
),
);
expect(mockGit.clone).not.toHaveBeenCalled();
});
it('should fallback to git clone without consent if no release data is found on first install', async () => {
mockDownloadFromGithubRelease.mockResolvedValue({
success: false,
failureReason: 'no release data',
type: 'github-release',
});
const requestConsent = vi.fn().mockResolvedValue(true);
await installOrUpdateExtension(
{ source: gitUrl, type: 'git' },
requestConsent,
);
// We should not see the request to use git clone, this is a repo that
// has no github releases so it is the only install method.
expect(requestConsent).toHaveBeenCalledExactlyOnceWith(
expect.stringContaining(
'Installing extension "gemini-test-extension"',
),
);
expect(mockGit.clone).toHaveBeenCalled();
const metadataPath = path.join(
userExtensionsDir,
extensionName,
INSTALL_METADATA_FILENAME,
);
const metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf-8'));
expect(metadata.type).toBe('git');
});
it('should ask for consent if no release data is found for an existing github-release extension', async () => {
mockDownloadFromGithubRelease.mockResolvedValue({
success: false,
failureReason: 'no release data',
errorMessage: 'No release data found',
type: 'github-release',
});
const requestConsent = vi.fn().mockResolvedValue(true);
await installOrUpdateExtension(
{ source: gitUrl, type: 'github-release' }, // Note the type
requestConsent,
);
expect(requestConsent).toHaveBeenCalledWith(
expect.stringContaining(
'Would you like to attempt to install via "git clone" instead?',
),
);
expect(mockGit.clone).toHaveBeenCalled();
});
});
});
describe('uninstallExtension', () => {

View File

@@ -464,16 +464,26 @@ export async function installOrUpdateExtension(
installMetadata.type === 'github-release'
) {
tempDir = await ExtensionStorage.createTmpDir();
try {
const result = await downloadFromGitHubRelease(
installMetadata,
tempDir,
);
const result = await downloadFromGitHubRelease(installMetadata, tempDir);
if (result.success) {
installMetadata.type = result.type;
installMetadata.releaseTag = result.tagName;
} catch (_error) {
} else if (
// This repo has no github releases, and wasn't explicitly installed
// from a github release, unconditionally just clone it.
(result.failureReason === 'no release data' &&
installMetadata.type === 'git') ||
// Otherwise ask the user if they would like to try a git clone.
(await requestConsent(
`Error downloading github release for ${installMetadata.source} with the following error: ${result.errorMessage}.\n\nWould you like to attempt to install via "git clone" instead?`,
))
) {
await cloneFromGit(installMetadata, tempDir);
installMetadata.type = 'git';
} else {
throw new Error(
`Failed to install extension ${installMetadata.source}: ${result.errorMessage}`,
);
}
localSourcePath = tempDir;
} else if (

View File

@@ -348,21 +348,21 @@ describe('git extension helpers', () => {
describe('parseGitHubRepoForReleases', () => {
it('should parse owner and repo from a full GitHub URL', () => {
const source = 'https://github.com/owner/repo.git';
const { owner, repo } = parseGitHubRepoForReleases(source);
const { owner, repo } = parseGitHubRepoForReleases(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should parse owner and repo from a full GitHub URL without .git', () => {
const source = 'https://github.com/owner/repo';
const { owner, repo } = parseGitHubRepoForReleases(source);
const { owner, repo } = parseGitHubRepoForReleases(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should parse owner and repo from a full GitHub URL with a trailing slash', () => {
const source = 'https://github.com/owner/repo/';
const { owner, repo } = parseGitHubRepoForReleases(source);
const { owner, repo } = parseGitHubRepoForReleases(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
@@ -374,23 +374,21 @@ describe('git extension helpers', () => {
);
});
it('should fail on a non-GitHub URL', () => {
it('should return null on a non-GitHub URL', () => {
const source = 'https://example.com/owner/repo.git';
expect(() => parseGitHubRepoForReleases(source)).toThrow(
'Invalid GitHub repository source: https://example.com/owner/repo.git. Expected "owner/repo" or a github repo uri.',
);
expect(parseGitHubRepoForReleases(source)).toBe(null);
});
it('should parse owner and repo from a shorthand string', () => {
const source = 'owner/repo';
const { owner, repo } = parseGitHubRepoForReleases(source);
const { owner, repo } = parseGitHubRepoForReleases(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should handle .git suffix in repo name', () => {
const source = 'owner/repo.git';
const { owner, repo } = parseGitHubRepoForReleases(source);
const { owner, repo } = parseGitHubRepoForReleases(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});

View File

@@ -79,16 +79,22 @@ export async function cloneFromGit(
export function parseGitHubRepoForReleases(source: string): {
owner: string;
repo: string;
} {
} | null {
// Default to a github repo path, so `source` can be just an org/repo
const parsedUrl = URL.parse(source, 'https://github.com');
if (!parsedUrl) {
throw new Error(`Invalid repo URL: ${source}`);
}
// The pathname should be "/owner/repo".
const parts = parsedUrl?.pathname
.substring(1)
.split('/')
// Remove the empty segments, fixes trailing slashes
.filter((part) => part !== '');
if (parts?.length !== 2 || parsedUrl?.host !== 'github.com') {
if (parsedUrl?.host !== 'github.com') {
return null;
}
if (parts?.length !== 2) {
throw new Error(
`Invalid GitHub repository source: ${source}. Expected "owner/repo" or a github repo uri.`,
);
@@ -110,7 +116,7 @@ export async function fetchReleaseFromGithub(
repo: string,
ref?: string,
allowPreRelease?: boolean,
): Promise<GithubReleaseData> {
): Promise<GithubReleaseData | null> {
if (ref) {
return await fetchJson(
`https://api.github.com/repos/${owner}/${repo}/releases/tags/${ref}`,
@@ -120,9 +126,14 @@ export async function fetchReleaseFromGithub(
if (!allowPreRelease) {
// Grab the release that is tagged as the "latest", github does not allow
// this to be a pre-release so we can blindly grab it.
return await fetchJson(
`https://api.github.com/repos/${owner}/${repo}/releases/latest`,
);
try {
return await fetchJson(
`https://api.github.com/repos/${owner}/${repo}/releases/latest`,
);
} catch (_) {
// This can fail if there is no release marked latest. In that case
// we want to just try the pre-release logic below.
}
}
// If pre-releases are allowed, we just grab the most recent release.
@@ -130,7 +141,7 @@ export async function fetchReleaseFromGithub(
`https://api.github.com/repos/${owner}/${repo}/releases?per_page=1`,
);
if (releases.length === 0) {
throw new Error('No releases found');
return null;
}
return releases[0];
}
@@ -206,7 +217,14 @@ export async function checkForExtensionUpdate(
console.error(`No "source" provided for extension.`);
return ExtensionUpdateState.ERROR;
}
const { owner, repo } = parseGitHubRepoForReleases(source);
const repoInfo = parseGitHubRepoForReleases(source);
if (!repoInfo) {
console.error(
`Source is not a valid GitHub repository for release checks: ${source}`,
);
return ExtensionUpdateState.ERROR;
}
const { owner, repo } = repoInfo;
const releaseData = await fetchReleaseFromGithub(
owner,
@@ -214,6 +232,9 @@ export async function checkForExtensionUpdate(
installMetadata.ref,
installMetadata.allowPreRelease,
);
if (!releaseData) {
return ExtensionUpdateState.ERROR;
}
if (releaseData.tag_name !== releaseTag) {
return ExtensionUpdateState.UPDATE_AVAILABLE;
}
@@ -226,28 +247,57 @@ export async function checkForExtensionUpdate(
return ExtensionUpdateState.ERROR;
}
}
export interface GitHubDownloadResult {
tagName: string;
tagName?: string;
type: 'git' | 'github-release';
success: boolean;
failureReason?:
| 'failed to fetch release data'
| 'no release data'
| 'no release asset found'
| 'failed to download asset'
| 'failed to extract asset'
| 'unknown';
errorMessage?: string;
}
export async function downloadFromGitHubRelease(
installMetadata: ExtensionInstallMetadata,
destination: string,
): Promise<GitHubDownloadResult> {
const { source, ref, allowPreRelease: preRelease } = installMetadata;
const { owner, repo } = parseGitHubRepoForReleases(source);
let releaseData: GithubReleaseData | null = null;
try {
const releaseData = await fetchReleaseFromGithub(
owner,
repo,
ref,
preRelease,
);
if (!releaseData) {
throw new Error(
`No release data found for ${owner}/${repo} at tag ${ref}`,
);
const parts = parseGitHubRepoForReleases(source);
if (!parts) {
return {
failureReason: 'no release data',
success: false,
type: 'github-release',
errorMessage: `Not a github repo: ${source}`,
};
}
const { owner, repo } = parts;
try {
releaseData = await fetchReleaseFromGithub(owner, repo, ref, preRelease);
if (!releaseData) {
return {
failureReason: 'no release data',
success: false,
type: 'github-release',
errorMessage: `No release data found for ${owner}/${repo} at tag ${ref}`,
};
}
} catch (error) {
return {
failureReason: 'failed to fetch release data',
success: false,
type: 'github-release',
errorMessage: `Failed to fetch release data for ${owner}/${repo} at tag ${ref}: ${getErrorMessage(error)}`,
};
}
const asset = findReleaseAsset(releaseData.assets);
@@ -266,9 +316,13 @@ export async function downloadFromGitHubRelease(
}
}
if (!archiveUrl) {
throw new Error(
`No assets found for release with tag ${releaseData.tag_name}`,
);
return {
failureReason: 'no release asset found',
success: false,
type: 'github-release',
tagName: releaseData.tag_name,
errorMessage: `No assets found for release with tag ${releaseData.tag_name}`,
};
}
let downloadedAssetPath = path.join(
destination,
@@ -280,9 +334,29 @@ export async function downloadFromGitHubRelease(
downloadedAssetPath += '.zip';
}
await downloadFile(archiveUrl, downloadedAssetPath);
try {
await downloadFile(archiveUrl, downloadedAssetPath);
} catch (error) {
return {
failureReason: 'failed to download asset',
success: false,
type: 'github-release',
tagName: releaseData.tag_name,
errorMessage: `Failed to download asset from ${archiveUrl}: ${getErrorMessage(error)}`,
};
}
await extractFile(downloadedAssetPath, destination);
try {
await extractFile(downloadedAssetPath, destination);
} catch (error) {
return {
failureReason: 'failed to extract asset',
success: false,
type: 'github-release',
tagName: releaseData.tag_name,
errorMessage: `Failed to extract asset from ${downloadedAssetPath}: ${getErrorMessage(error)}`,
};
}
// For regular github releases, the repository is put inside of a top level
// directory. In this case we should see exactly two file in the destination
@@ -316,11 +390,16 @@ export async function downloadFromGitHubRelease(
return {
tagName: releaseData.tag_name,
type: 'github-release',
success: true,
};
} catch (error) {
throw new Error(
`Failed to download release from ${installMetadata.source}: ${getErrorMessage(error)}`,
);
return {
failureReason: 'unknown',
success: false,
type: 'github-release',
tagName: releaseData?.tag_name,
errorMessage: `Failed to download release from ${installMetadata.source}: ${getErrorMessage(error)}`,
};
}
}