mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-26 04:54:25 -07:00
Fix rough edges around extension updates (#10926)
This commit is contained in:
@@ -15,7 +15,7 @@ import {
|
||||
annotateActiveExtensions,
|
||||
disableExtension,
|
||||
enableExtension,
|
||||
installExtension,
|
||||
installOrUpdateExtension,
|
||||
loadExtension,
|
||||
loadExtensionConfig,
|
||||
loadExtensions,
|
||||
@@ -73,6 +73,7 @@ vi.mock('./trustedFolders.js', async (importOriginal) => {
|
||||
const mockLogExtensionEnable = vi.hoisted(() => vi.fn());
|
||||
const mockLogExtensionInstallEvent = vi.hoisted(() => vi.fn());
|
||||
const mockLogExtensionUninstall = vi.hoisted(() => vi.fn());
|
||||
const mockLogExtensionUpdateEvent = vi.hoisted(() => vi.fn());
|
||||
const mockLogExtensionDisable = vi.hoisted(() => vi.fn());
|
||||
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
const actual =
|
||||
@@ -82,6 +83,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
logExtensionEnable: mockLogExtensionEnable,
|
||||
logExtensionInstallEvent: mockLogExtensionInstallEvent,
|
||||
logExtensionUninstall: mockLogExtensionUninstall,
|
||||
logExtensionUpdateEvent: mockLogExtensionUpdateEvent,
|
||||
logExtensionDisable: mockLogExtensionDisable,
|
||||
ExtensionEnableEvent: vi.fn(),
|
||||
ExtensionInstallEvent: vi.fn(),
|
||||
@@ -260,7 +262,7 @@ describe('extension tests', () => {
|
||||
});
|
||||
fs.writeFileSync(path.join(sourceExtDir, 'context.md'), 'linked context');
|
||||
|
||||
const extensionName = await installExtension(
|
||||
const extensionName = await installOrUpdateExtension(
|
||||
{
|
||||
source: sourceExtDir,
|
||||
type: 'link',
|
||||
@@ -703,7 +705,7 @@ describe('extension tests', () => {
|
||||
const targetExtDir = path.join(userExtensionsDir, 'my-local-extension');
|
||||
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
|
||||
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
);
|
||||
@@ -724,12 +726,12 @@ describe('extension tests', () => {
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
});
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
);
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
),
|
||||
@@ -744,7 +746,7 @@ describe('extension tests', () => {
|
||||
const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME);
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
),
|
||||
@@ -761,7 +763,7 @@ describe('extension tests', () => {
|
||||
fs.writeFileSync(configPath, '{ "name": "bad-json", "version": "1.0.0"'); // Malformed JSON
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
),
|
||||
@@ -786,7 +788,7 @@ describe('extension tests', () => {
|
||||
fs.writeFileSync(configPath, JSON.stringify({ version: '1.0.0' }));
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
),
|
||||
@@ -812,7 +814,7 @@ describe('extension tests', () => {
|
||||
});
|
||||
mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]);
|
||||
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
{ source: gitUrl, type: 'git' },
|
||||
async (_) => true,
|
||||
);
|
||||
@@ -836,7 +838,7 @@ describe('extension tests', () => {
|
||||
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
|
||||
const configPath = path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME);
|
||||
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'link' },
|
||||
async (_) => true,
|
||||
);
|
||||
@@ -854,20 +856,77 @@ describe('extension tests', () => {
|
||||
fs.rmSync(targetExtDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('should log to clearcut on successful install', async () => {
|
||||
const sourceExtDir = createExtension({
|
||||
extensionsDir: tempHomeDir,
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
});
|
||||
describe.each([true, false])(
|
||||
'with previous extension config: %s',
|
||||
(isUpdate: boolean) => {
|
||||
let sourceExtDir: string;
|
||||
|
||||
await installExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
);
|
||||
beforeEach(async () => {
|
||||
sourceExtDir = createExtension({
|
||||
extensionsDir: tempHomeDir,
|
||||
name: 'my-local-extension',
|
||||
version: '1.1.0',
|
||||
});
|
||||
if (isUpdate) {
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
);
|
||||
}
|
||||
// Clears out any calls to mocks from the above function calls.
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
expect(mockLogExtensionInstallEvent).toHaveBeenCalled();
|
||||
});
|
||||
it(`should log an ${isUpdate ? 'update' : 'install'} event to clearcut on success`, async () => {
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
undefined,
|
||||
isUpdate
|
||||
? {
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
}
|
||||
: undefined,
|
||||
);
|
||||
|
||||
if (isUpdate) {
|
||||
expect(mockLogExtensionUpdateEvent).toHaveBeenCalled();
|
||||
expect(mockLogExtensionInstallEvent).not.toHaveBeenCalled();
|
||||
} else {
|
||||
expect(mockLogExtensionInstallEvent).toHaveBeenCalled();
|
||||
expect(mockLogExtensionUpdateEvent).not.toHaveBeenCalled();
|
||||
}
|
||||
});
|
||||
|
||||
it(`should ${isUpdate ? 'not ' : ''} alter the extension enablement configuration`, async () => {
|
||||
const enablementManager = new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
);
|
||||
enablementManager.enable('my-local-extension', true, '/some/scope');
|
||||
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
undefined,
|
||||
isUpdate
|
||||
? {
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
}
|
||||
: undefined,
|
||||
);
|
||||
|
||||
const config = enablementManager.readConfig()['my-local-extension'];
|
||||
if (isUpdate) {
|
||||
expect(config).not.toBeUndefined();
|
||||
expect(config.overrides).toContain('/some/scope/*');
|
||||
} else {
|
||||
expect(config).not.toContain('/some/scope/*');
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it('should show users information on their ansi escaped mcp servers when installing', async () => {
|
||||
const sourceExtDir = createExtension({
|
||||
@@ -891,7 +950,7 @@ describe('extension tests', () => {
|
||||
mockRequestConsent.mockResolvedValue(true);
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
mockRequestConsent,
|
||||
),
|
||||
@@ -920,7 +979,7 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async () => true,
|
||||
),
|
||||
@@ -941,7 +1000,7 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async () => false,
|
||||
),
|
||||
@@ -957,7 +1016,7 @@ This extension will run the following MCP servers:
|
||||
const targetExtDir = path.join(userExtensionsDir, 'my-local-extension');
|
||||
const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME);
|
||||
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
{
|
||||
source: sourceExtDir,
|
||||
type: 'local',
|
||||
@@ -991,9 +1050,15 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
const mockRequestConsent = vi.fn();
|
||||
// Install it and force consent first.
|
||||
await installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async () => true,
|
||||
);
|
||||
|
||||
// Now update it without changing anything.
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
mockRequestConsent,
|
||||
process.cwd(),
|
||||
@@ -1016,7 +1081,7 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
await expect(
|
||||
installExtension(
|
||||
installOrUpdateExtension(
|
||||
{ source: sourceExtDir, type: 'local' },
|
||||
async (_) => true,
|
||||
),
|
||||
@@ -1032,7 +1097,7 @@ This extension will run the following MCP servers:
|
||||
version: '1.0.0',
|
||||
});
|
||||
|
||||
await uninstallExtension('my-local-extension');
|
||||
await uninstallExtension('my-local-extension', false);
|
||||
|
||||
expect(fs.existsSync(sourceExtDir)).toBe(false);
|
||||
});
|
||||
@@ -1049,7 +1114,7 @@ This extension will run the following MCP servers:
|
||||
version: '1.0.0',
|
||||
});
|
||||
|
||||
await uninstallExtension('my-local-extension');
|
||||
await uninstallExtension('my-local-extension', false);
|
||||
|
||||
expect(fs.existsSync(sourceExtDir)).toBe(false);
|
||||
expect(
|
||||
@@ -1063,25 +1128,54 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
it('should throw an error if the extension does not exist', async () => {
|
||||
await expect(uninstallExtension('nonexistent-extension')).rejects.toThrow(
|
||||
'Extension not found.',
|
||||
);
|
||||
await expect(
|
||||
uninstallExtension('nonexistent-extension', false),
|
||||
).rejects.toThrow('Extension not found.');
|
||||
});
|
||||
|
||||
it('should log uninstall event', async () => {
|
||||
createExtension({
|
||||
extensionsDir: userExtensionsDir,
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
describe.each([true, false])('with isUpdate: %s', (isUpdate: boolean) => {
|
||||
it(`should ${isUpdate ? 'not ' : ''}log uninstall event`, async () => {
|
||||
createExtension({
|
||||
extensionsDir: userExtensionsDir,
|
||||
name: 'my-local-extension',
|
||||
version: '1.0.0',
|
||||
});
|
||||
|
||||
await uninstallExtension('my-local-extension', isUpdate);
|
||||
|
||||
if (isUpdate) {
|
||||
expect(mockLogExtensionUninstall).not.toHaveBeenCalled();
|
||||
expect(ExtensionUninstallEvent).not.toHaveBeenCalled();
|
||||
} else {
|
||||
expect(mockLogExtensionUninstall).toHaveBeenCalled();
|
||||
expect(ExtensionUninstallEvent).toHaveBeenCalledWith(
|
||||
'my-local-extension',
|
||||
'success',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
await uninstallExtension('my-local-extension');
|
||||
it(`should ${isUpdate ? 'not ' : ''} alter the extension enablement configuration`, async () => {
|
||||
createExtension({
|
||||
extensionsDir: userExtensionsDir,
|
||||
name: 'test-extension',
|
||||
version: '1.0.0',
|
||||
});
|
||||
const enablementManager = new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
);
|
||||
enablementManager.enable('test-extension', true, '/some/scope');
|
||||
|
||||
expect(mockLogExtensionUninstall).toHaveBeenCalled();
|
||||
expect(ExtensionUninstallEvent).toHaveBeenCalledWith(
|
||||
'my-local-extension',
|
||||
'success',
|
||||
);
|
||||
await uninstallExtension('test-extension', isUpdate);
|
||||
|
||||
const config = enablementManager.readConfig()['test-extension'];
|
||||
if (isUpdate) {
|
||||
expect(config).not.toBeUndefined();
|
||||
expect(config.overrides).toEqual(['/some/scope/*']);
|
||||
} else {
|
||||
expect(config).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('should uninstall an extension by its source URL', async () => {
|
||||
@@ -1096,7 +1190,7 @@ This extension will run the following MCP servers:
|
||||
},
|
||||
});
|
||||
|
||||
await uninstallExtension(gitUrl);
|
||||
await uninstallExtension(gitUrl, false);
|
||||
|
||||
expect(fs.existsSync(sourceExtDir)).toBe(false);
|
||||
expect(mockLogExtensionUninstall).toHaveBeenCalled();
|
||||
@@ -1115,7 +1209,10 @@ This extension will run the following MCP servers:
|
||||
});
|
||||
|
||||
await expect(
|
||||
uninstallExtension('https://github.com/google/no-metadata-extension'),
|
||||
uninstallExtension(
|
||||
'https://github.com/google/no-metadata-extension',
|
||||
false,
|
||||
),
|
||||
).rejects.toThrow('Extension not found.');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,11 +15,13 @@ import {
|
||||
Config,
|
||||
ExtensionInstallEvent,
|
||||
ExtensionUninstallEvent,
|
||||
ExtensionUpdateEvent,
|
||||
ExtensionDisableEvent,
|
||||
ExtensionEnableEvent,
|
||||
logExtensionEnable,
|
||||
logExtensionInstallEvent,
|
||||
logExtensionUninstall,
|
||||
logExtensionUpdateEvent,
|
||||
logExtensionDisable,
|
||||
} from '@google/gemini-cli-core';
|
||||
import * as fs from 'node:fs';
|
||||
@@ -129,7 +131,7 @@ export async function performWorkspaceExtensionMigration(
|
||||
source: extension.path,
|
||||
type: 'local',
|
||||
};
|
||||
await installExtension(installMetadata, requestConsent);
|
||||
await installOrUpdateExtension(installMetadata, requestConsent);
|
||||
} catch (_) {
|
||||
failedInstallNames.push(extension.name);
|
||||
}
|
||||
@@ -426,12 +428,13 @@ async function promptForConsentInteractive(
|
||||
});
|
||||
}
|
||||
|
||||
export async function installExtension(
|
||||
export async function installOrUpdateExtension(
|
||||
installMetadata: ExtensionInstallMetadata,
|
||||
requestConsent: (consent: string) => Promise<boolean>,
|
||||
cwd: string = process.cwd(),
|
||||
previousExtensionConfig?: ExtensionConfig,
|
||||
): Promise<string> {
|
||||
const isUpdate = !!previousExtensionConfig;
|
||||
const telemetryConfig = getTelemetryConfig(cwd);
|
||||
let newExtensionConfig: ExtensionConfig | null = null;
|
||||
let localSourcePath: string | undefined;
|
||||
@@ -489,24 +492,32 @@ export async function installExtension(
|
||||
});
|
||||
|
||||
const newExtensionName = newExtensionConfig.name;
|
||||
const extensionStorage = new ExtensionStorage(newExtensionName);
|
||||
const destinationPath = extensionStorage.getExtensionDir();
|
||||
|
||||
const installedExtensions = loadUserExtensions();
|
||||
if (
|
||||
installedExtensions.some(
|
||||
(installed) => installed.name === newExtensionName,
|
||||
)
|
||||
) {
|
||||
throw new Error(
|
||||
`Extension "${newExtensionName}" is already installed. Please uninstall it first.`,
|
||||
);
|
||||
if (!isUpdate) {
|
||||
const installedExtensions = loadUserExtensions();
|
||||
if (
|
||||
installedExtensions.some(
|
||||
(installed) => installed.name === newExtensionName,
|
||||
)
|
||||
) {
|
||||
throw new Error(
|
||||
`Extension "${newExtensionName}" is already installed. Please uninstall it first.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
await maybeRequestConsentOrFail(
|
||||
newExtensionConfig,
|
||||
requestConsent,
|
||||
previousExtensionConfig,
|
||||
);
|
||||
|
||||
const extensionStorage = new ExtensionStorage(newExtensionName);
|
||||
const destinationPath = extensionStorage.getExtensionDir();
|
||||
|
||||
if (isUpdate) {
|
||||
await uninstallExtension(newExtensionName, isUpdate, cwd);
|
||||
}
|
||||
|
||||
await fs.promises.mkdir(destinationPath, { recursive: true });
|
||||
|
||||
if (
|
||||
@@ -529,17 +540,30 @@ export async function installExtension(
|
||||
}
|
||||
}
|
||||
|
||||
logExtensionInstallEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionInstallEvent(
|
||||
newExtensionConfig!.name,
|
||||
newExtensionConfig!.version,
|
||||
installMetadata.source,
|
||||
'success',
|
||||
),
|
||||
);
|
||||
if (isUpdate) {
|
||||
logExtensionUpdateEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionUpdateEvent(
|
||||
newExtensionConfig.name,
|
||||
newExtensionConfig.version,
|
||||
previousExtensionConfig.version,
|
||||
installMetadata.source,
|
||||
'success',
|
||||
),
|
||||
);
|
||||
} else {
|
||||
logExtensionInstallEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionInstallEvent(
|
||||
newExtensionConfig.name,
|
||||
newExtensionConfig.version,
|
||||
installMetadata.source,
|
||||
'success',
|
||||
),
|
||||
);
|
||||
enableExtension(newExtensionConfig.name, SettingScope.User);
|
||||
}
|
||||
|
||||
enableExtension(newExtensionConfig!.name, SettingScope.User);
|
||||
return newExtensionConfig!.name;
|
||||
} catch (error) {
|
||||
// Attempt to load config from the source path even if installation fails
|
||||
@@ -554,15 +578,28 @@ export async function installExtension(
|
||||
// Ignore error, this is just for logging.
|
||||
}
|
||||
}
|
||||
logExtensionInstallEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionInstallEvent(
|
||||
newExtensionConfig?.name ?? '',
|
||||
newExtensionConfig?.version ?? '',
|
||||
installMetadata.source,
|
||||
'error',
|
||||
),
|
||||
);
|
||||
if (isUpdate) {
|
||||
logExtensionUpdateEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionUpdateEvent(
|
||||
newExtensionConfig?.name ?? previousExtensionConfig.name,
|
||||
newExtensionConfig?.version ?? '',
|
||||
previousExtensionConfig.version,
|
||||
installMetadata.source,
|
||||
'error',
|
||||
),
|
||||
);
|
||||
} else {
|
||||
logExtensionInstallEvent(
|
||||
telemetryConfig,
|
||||
new ExtensionInstallEvent(
|
||||
newExtensionConfig?.name ?? '',
|
||||
newExtensionConfig?.version ?? '',
|
||||
installMetadata.source,
|
||||
'error',
|
||||
),
|
||||
);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
@@ -679,9 +716,9 @@ export function loadExtensionConfig(
|
||||
|
||||
export async function uninstallExtension(
|
||||
extensionIdentifier: string,
|
||||
isUpdate: boolean,
|
||||
cwd: string = process.cwd(),
|
||||
): Promise<void> {
|
||||
const telemetryConfig = getTelemetryConfig(cwd);
|
||||
const installedExtensions = loadUserExtensions();
|
||||
const extensionName = installedExtensions.find(
|
||||
(installed) =>
|
||||
@@ -692,17 +729,24 @@ export async function uninstallExtension(
|
||||
if (!extensionName) {
|
||||
throw new Error(`Extension not found.`);
|
||||
}
|
||||
const manager = new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
[extensionName],
|
||||
);
|
||||
manager.remove(extensionName);
|
||||
const storage = new ExtensionStorage(extensionName);
|
||||
|
||||
await fs.promises.rm(storage.getExtensionDir(), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
|
||||
// The rest of the cleanup below here is only for true uninstalls, not
|
||||
// uninstalls related to updates.
|
||||
if (isUpdate) return;
|
||||
|
||||
const manager = new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
[extensionName],
|
||||
);
|
||||
manager.remove(extensionName);
|
||||
|
||||
const telemetryConfig = getTelemetryConfig(cwd);
|
||||
logExtensionUninstall(
|
||||
telemetryConfig,
|
||||
new ExtensionUninstallEvent(extensionName, 'success'),
|
||||
|
||||
@@ -11,8 +11,7 @@ import {
|
||||
} from '../../ui/state/extensions.js';
|
||||
import {
|
||||
copyExtension,
|
||||
installExtension,
|
||||
uninstallExtension,
|
||||
installOrUpdateExtension,
|
||||
loadExtension,
|
||||
loadInstallMetadata,
|
||||
ExtensionStorage,
|
||||
@@ -65,13 +64,11 @@ export async function updateExtension(
|
||||
|
||||
const tempDir = await ExtensionStorage.createTmpDir();
|
||||
try {
|
||||
await copyExtension(extension.path, tempDir);
|
||||
const previousExtensionConfig = await loadExtensionConfig({
|
||||
extensionDir: extension.path,
|
||||
workspaceDir: cwd,
|
||||
});
|
||||
await uninstallExtension(extension.name, cwd);
|
||||
await installExtension(
|
||||
await installOrUpdateExtension(
|
||||
installMetadata,
|
||||
requestConsent,
|
||||
cwd,
|
||||
|
||||
Reference in New Issue
Block a user