MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-03-22 03:51:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(extensions): revert broken extension removal behavior (#23317)

Browse Source
This commit is contained in:
Emily Hedlund
2026-03-20 19:36:52 -04:00
committed by GitHub
parent 974d29128f
commit fc03891a11
3 changed files with 22 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
packages/cli/src/config/extension-manager.test.ts
View File

@@ -637,64 +637,4 @@ describe('ExtensionManager', () => {
);
});
});
describe('orphaned extension cleanup', () => {
it('should remove broken extension metadata on startup to allow re-installation', async () => {
const extName = 'orphaned-ext';
const sourceDir = path.join(tempHomeDir, 'valid-source');
fs.mkdirSync(sourceDir, { recursive: true });
fs.writeFileSync(
path.join(sourceDir, 'gemini-extension.json'),
JSON.stringify({ name: extName, version: '1.0.0' }),
);
// Link an extension successfully.
await extensionManager.loadExtensions();
await extensionManager.installOrUpdateExtension({
source: sourceDir,
type: 'link',
});
const destinationPath = path.join(userExtensionsDir, extName);
const metadataPath = path.join(
destinationPath,
'.gemini-extension-install.json',
);
expect(fs.existsSync(metadataPath)).toBe(true);
// Simulate metadata corruption (e.g., pointing to a non-existent source).
fs.writeFileSync(
metadataPath,
JSON.stringify({ source: '/NON_EXISTENT_PATH', type: 'link' }),
);
// Simulate CLI startup. The manager should detect the broken link
// and proactively delete the orphaned metadata directory.
const newManager = new ExtensionManager({
settings: createTestMergedSettings(),
workspaceDir: tempWorkspaceDir,
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: null,
integrityManager: mockIntegrityManager,
});
await newManager.loadExtensions();
// Verify the extension failed to load and was proactively cleaned up.
expect(newManager.getExtensions().some((e) => e.name === extName)).toBe(
false,
);
expect(fs.existsSync(destinationPath)).toBe(false);
// Verify the system is self-healed and allows re-linking to the valid source.
await newManager.installOrUpdateExtension({
source: sourceDir,
type: 'link',
});
expect(newManager.getExtensions().some((e) => e.name === extName)).toBe(
true,
);
});
});
});

15
packages/cli/src/config/extension-manager.ts
View File

@@ -982,18 +982,11 @@ Would you like to attempt to install via "git clone" instead?`,
plan: config.plan,
};
} catch (e) {
const extName = path.basename(extensionDir);
debugLogger.warn(
`Warning: Removing broken extension ${extName}: ${getErrorMessage(e)}`,
debugLogger.error(
`Warning: Skipping extension in ${effectiveExtensionPath}: ${getErrorMessage(
e,
)}`,
);
try {
await fs.promises.rm(extensionDir, { recursive: true, force: true });
} catch (rmError) {
debugLogger.error(
`Failed to remove broken extension directory ${extensionDir}:`,
rmError,
);
}
return null;
}
}

28
packages/cli/src/config/extension.test.ts
View File

@@ -249,8 +249,10 @@ describe('extension tests', () => {
expect(extensions[0].name).toBe('test-extension');
});
it('should log a warning and remove the extension if a context file path is outside the extension directory', async () => {
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
it('should skip the extension if a context file path is outside the extension directory and log an error', async () => {
const consoleSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
createExtension({
extensionsDir: userExtensionsDir,
name: 'traversal-extension',
@@ -660,8 +662,10 @@ name = "yolo-checker"
expect(serverConfig.env!['MISSING_VAR_BRACES']).toBe('${ALSO_UNDEFINED}');
});
it('should remove an extension with invalid JSON config and log a warning', async () => {
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
it('should skip an extension with invalid JSON config and log an error', async () => {
const consoleSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Good extension
createExtension({
@@ -682,15 +686,17 @@ name = "yolo-checker"
expect(extensions[0].name).toBe('good-ext');
expect(consoleSpy).toHaveBeenCalledWith(
expect.stringContaining(
`Warning: Removing broken extension bad-ext: Failed to load extension config from ${badConfigPath}`,
`Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}`,
),
);
consoleSpy.mockRestore();
});
it('should remove an extension with missing "name" in config and log a warning', async () => {
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
it('should skip an extension with missing "name" in config and log an error', async () => {
const consoleSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Good extension
createExtension({
@@ -711,7 +717,7 @@ name = "yolo-checker"
expect(extensions[0].name).toBe('good-ext');
expect(consoleSpy).toHaveBeenCalledWith(
expect.stringContaining(
`Warning: Removing broken extension bad-ext-no-name: Failed to load extension config from ${badConfigPath}: Invalid configuration in ${badConfigPath}: missing "name"`,
`Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}: Invalid configuration in ${badConfigPath}: missing "name"`,
),
);
@@ -737,8 +743,10 @@ name = "yolo-checker"
expect(extensions[0].mcpServers?.['test-server'].trust).toBeUndefined();
});
it('should log a warning for invalid extension names during loading', async () => {
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
it('should log an error for invalid extension names during loading', async () => {
const consoleSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
createExtension({
extensionsDir: userExtensionsDir,
name: 'bad_name',