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

Author: ehedlund

packages/cli/src/config/extension-manager.test.ts

@@ -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,
-      );
-    });
-  });
 });

packages/cli/src/config/extension-manager.ts

@@ -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;
     }
   }

packages/cli/src/config/extension.test.ts

@@ -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',