Inform user of missing settings on extensions update (#15944)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

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

@@ -46,6 +46,7 @@ import {
   type HookDefinition,
   type HookEventName,
   type ResolvedExtensionSetting,
+  coreEvents,
 } from '@google/gemini-cli-core';
 import { maybeRequestConsentOrFail } from './extensions/consent.js';
 import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
@@ -59,6 +60,7 @@ import {
 import {
   getEnvContents,
   maybePromptForSettings,
+  getMissingSettings,
   type ExtensionSetting,
 } from './extensions/extensionSettings.js';
 import type { EventEmitter } from 'node:stream';
@@ -227,25 +229,6 @@ Would you like to attempt to install via "git clone" instead?`,
       try {
         newExtensionConfig = await this.loadExtensionConfig(localSourcePath);
 
-        if (isUpdate && installMetadata.autoUpdate) {
-          const oldSettings = new Set(
-            previousExtensionConfig.settings?.map((s) => s.name) || [],
-          );
-          const newSettings = new Set(
-            newExtensionConfig.settings?.map((s) => s.name) || [],
-          );
-
-          const settingsAreEqual =
-            oldSettings.size === newSettings.size &&
-            [...oldSettings].every((value) => newSettings.has(value));
-
-          if (!settingsAreEqual && installMetadata.autoUpdate) {
-            throw new Error(
-              `Extension "${newExtensionConfig.name}" has settings changes and cannot be auto-updated. Please update manually.`,
-            );
-          }
-        }
-
         const newExtensionName = newExtensionConfig.name;
         const previous = this.getExtensions().find(
           (installed) => installed.name === newExtensionName,
@@ -316,6 +299,20 @@ Would you like to attempt to install via "git clone" instead?`,
           }
         }
 
+        const missingSettings = await getMissingSettings(
+          newExtensionConfig,
+          extensionId,
+        );
+        if (missingSettings.length > 0) {
+          const message = `Extension "${newExtensionConfig.name}" has missing settings: ${missingSettings
+            .map((s) => s.name)
+            .join(
+              ', ',
+            )}. Please run "gemini extensions settings ${newExtensionConfig.name} <setting-name>" to configure them.`;
+          debugLogger.warn(message);
+          coreEvents.emitFeedback('warning', message);
+        }
+
         if (
           installMetadata.type === 'local' ||
           installMetadata.type === 'git' ||

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

@@ -1447,7 +1447,7 @@ This extension will run the following MCP servers:
       expect(envContent).toContain('NEW_SETTING=new-setting-value');
     });
 
-    it('should fail auto-update if settings have changed', async () => {
+    it('should auto-update if settings have changed', async () => {
       // 1. Install initial version with autoUpdate: true
       const oldSourceExtDir = createExtension({
         extensionsDir: tempHomeDir,
@@ -1469,7 +1469,7 @@ This extension will run the following MCP servers:
       });
 
       // 2. Create new version with different settings
-      const newSourceExtDir = createExtension({
+      const extensionDir = createExtension({
         extensionsDir: tempHomeDir,
         name: 'my-auto-update-ext',
         version: '1.1.0',
@@ -1488,14 +1488,17 @@ This extension will run the following MCP servers:
         );
 
       // 3. Attempt to update and assert it fails
-      await expect(
-        extensionManager.installOrUpdateExtension(
-          { source: newSourceExtDir, type: 'local', autoUpdate: true },
-          previousExtensionConfig,
-        ),
-      ).rejects.toThrow(
-        'Extension "my-auto-update-ext" has settings changes and cannot be auto-updated. Please update manually.',
+      const updatedExtension = await extensionManager.installOrUpdateExtension(
+        {
+          source: extensionDir,
+          type: 'local',
+          autoUpdate: true,
+        },
+        previousExtensionConfig,
       );
+
+      expect(updatedExtension.version).toBe('1.1.0');
+      expect(extensionManager.getExtensions()[0].version).toBe('1.1.0');
     });
 
     it('should throw an error for invalid extension names', async () => {

packages/cli/src/config/extensions/extensionSettings.ts

@@ -298,3 +298,24 @@ async function clearSettings(
   }
   return;
 }
+
+export async function getMissingSettings(
+  extensionConfig: ExtensionConfig,
+  extensionId: string,
+): Promise<ExtensionSetting[]> {
+  const { settings } = extensionConfig;
+  if (!settings || settings.length === 0) {
+    return [];
+  }
+
+  const existingSettings = await getEnvContents(extensionConfig, extensionId);
+  const missingSettings: ExtensionSetting[] = [];
+
+  for (const setting of settings) {
+    if (existingSettings[setting.envVar] === undefined) {
+      missingSettings.push(setting);
+    }
+  }
+
+  return missingSettings;
+}

packages/cli/src/config/extensions/extensionUpdates.test.ts

@@ -0,0 +1,308 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import * as fs from 'node:fs';
+import { getMissingSettings } from './extensionSettings.js';
+import type { ExtensionConfig } from '../extension.js';
+import { ExtensionStorage } from './storage.js';
+import {
+  KeychainTokenStorage,
+  debugLogger,
+  type ExtensionInstallMetadata,
+  type GeminiCLIExtension,
+  coreEvents,
+} from '@google/gemini-cli-core';
+import { EXTENSION_SETTINGS_FILENAME } from './variables.js';
+import { ExtensionManager } from '../extension-manager.js';
+
+vi.mock('node:fs', async (importOriginal) => {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const actual = await importOriginal<any>();
+  return {
+    ...actual,
+    default: {
+      ...actual.default,
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      existsSync: vi.fn((...args: any[]) => actual.existsSync(...args)),
+    },
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    existsSync: vi.fn((...args: any[]) => actual.existsSync(...args)),
+  };
+});
+
+vi.mock('@google/gemini-cli-core', async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import('@google/gemini-cli-core')>();
+  return {
+    ...actual,
+    KeychainTokenStorage: vi.fn(),
+    debugLogger: {
+      warn: vi.fn(),
+      error: vi.fn(),
+      log: vi.fn(),
+    },
+    coreEvents: {
+      emitFeedback: vi.fn(), // Mock emitFeedback
+      on: vi.fn(),
+      off: vi.fn(),
+    },
+  };
+});
+
+// Mock os.homedir because ExtensionStorage uses it
+vi.mock('os', async (importOriginal) => {
+  const mockedOs = await importOriginal<typeof os>();
+  return {
+    ...mockedOs,
+    homedir: vi.fn(),
+  };
+});
+
+describe('extensionUpdates', () => {
+  let tempHomeDir: string;
+  let tempWorkspaceDir: string;
+  let extensionDir: string;
+  let mockKeychainData: Record<string, Record<string, string>>;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockKeychainData = {};
+
+    // Mock Keychain
+    vi.mocked(KeychainTokenStorage).mockImplementation(
+      (serviceName: string) => {
+        if (!mockKeychainData[serviceName]) {
+          mockKeychainData[serviceName] = {};
+        }
+        const keychainData = mockKeychainData[serviceName];
+        return {
+          getSecret: vi
+            .fn()
+            .mockImplementation(
+              async (key: string) => keychainData[key] || null,
+            ),
+          setSecret: vi
+            .fn()
+            .mockImplementation(async (key: string, value: string) => {
+              keychainData[key] = value;
+            }),
+          deleteSecret: vi.fn().mockImplementation(async (key: string) => {
+            delete keychainData[key];
+          }),
+          listSecrets: vi
+            .fn()
+            .mockImplementation(async () => Object.keys(keychainData)),
+          isAvailable: vi.fn().mockResolvedValue(true),
+        } as unknown as KeychainTokenStorage;
+      },
+    );
+
+    // Setup Temp Dirs
+    tempHomeDir = fs.mkdtempSync(
+      path.join(os.tmpdir(), 'gemini-cli-test-home-'),
+    );
+    tempWorkspaceDir = fs.mkdtempSync(
+      path.join(os.tmpdir(), 'gemini-cli-test-workspace-'),
+    );
+    extensionDir = path.join(tempHomeDir, '.gemini', 'extensions', 'test-ext');
+
+    // Mock ExtensionStorage to rely on our temp extension dir
+    vi.spyOn(ExtensionStorage.prototype, 'getExtensionDir').mockReturnValue(
+      extensionDir,
+    );
+    // Mock getEnvFilePath is checking extensionDir/variables.env? No, it used ExtensionStorage logic.
+    // getEnvFilePath in extensionSettings.ts:
+    // if workspace, process.cwd()/.env (we need to mock process.cwd or move tempWorkspaceDir there)
+    // if user, ExtensionStorage(name).getEnvFilePath() -> joins extensionDir + '.env'
+
+    fs.mkdirSync(extensionDir, { recursive: true });
+    vi.mocked(os.homedir).mockReturnValue(tempHomeDir);
+    vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir);
+  });
+
+  afterEach(() => {
+    fs.rmSync(tempHomeDir, { recursive: true, force: true });
+    fs.rmSync(tempWorkspaceDir, { recursive: true, force: true });
+    vi.restoreAllMocks();
+  });
+
+  describe('getMissingSettings', () => {
+    it('should return empty list if all settings are present', async () => {
+      const config: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.0.0',
+        settings: [
+          { name: 's1', description: 'd1', envVar: 'VAR1' },
+          { name: 's2', description: 'd2', envVar: 'VAR2', sensitive: true },
+        ],
+      };
+      const extensionId = '12345';
+
+      // Setup User Env
+      const userEnvPath = path.join(extensionDir, EXTENSION_SETTINGS_FILENAME);
+      fs.writeFileSync(userEnvPath, 'VAR1=val1');
+
+      // Setup Keychain
+      const userKeychain = new KeychainTokenStorage(
+        `Gemini CLI Extensions test-ext ${extensionId}`,
+      );
+      await userKeychain.setSecret('VAR2', 'val2');
+
+      const missing = await getMissingSettings(config, extensionId);
+      expect(missing).toEqual([]);
+    });
+
+    it('should identify missing non-sensitive settings', async () => {
+      const config: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.0.0',
+        settings: [{ name: 's1', description: 'd1', envVar: 'VAR1' }],
+      };
+      const extensionId = '12345';
+
+      const missing = await getMissingSettings(config, extensionId);
+      expect(missing).toHaveLength(1);
+      expect(missing[0].name).toBe('s1');
+    });
+
+    it('should identify missing sensitive settings', async () => {
+      const config: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.0.0',
+        settings: [
+          { name: 's2', description: 'd2', envVar: 'VAR2', sensitive: true },
+        ],
+      };
+      const extensionId = '12345';
+
+      const missing = await getMissingSettings(config, extensionId);
+      expect(missing).toHaveLength(1);
+      expect(missing[0].name).toBe('s2');
+    });
+
+    it('should respect settings present in workspace', async () => {
+      const config: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.0.0',
+        settings: [{ name: 's1', description: 'd1', envVar: 'VAR1' }],
+      };
+      const extensionId = '12345';
+
+      // Setup Workspace Env
+      const workspaceEnvPath = path.join(
+        tempWorkspaceDir,
+        EXTENSION_SETTINGS_FILENAME,
+      );
+      fs.writeFileSync(workspaceEnvPath, 'VAR1=val1');
+
+      const missing = await getMissingSettings(config, extensionId);
+      expect(missing).toEqual([]);
+    });
+  });
+
+  describe('ExtensionManager integration', () => {
+    it('should warn about missing settings after update', async () => {
+      // Mock ExtensionManager methods to avoid FS/Network usage
+      const newConfig: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.1.0',
+        settings: [{ name: 's1', description: 'd1', envVar: 'VAR1' }],
+      };
+
+      const previousConfig: ExtensionConfig = {
+        name: 'test-ext',
+        version: '1.0.0',
+        settings: [],
+      };
+
+      const installMetadata: ExtensionInstallMetadata = {
+        source: extensionDir,
+        type: 'local',
+        autoUpdate: true,
+      };
+
+      const manager = new ExtensionManager({
+        workspaceDir: tempWorkspaceDir,
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        settings: { telemetry: {} } as any,
+        requestConsent: vi.fn().mockResolvedValue(true),
+        requestSetting: null, // Simulate non-interactive
+      });
+
+      // Mock methods called by installOrUpdateExtension
+      vi.spyOn(manager, 'loadExtensionConfig').mockResolvedValue(newConfig);
+      vi.spyOn(manager, 'getExtensions').mockReturnValue([
+        {
+          name: 'test-ext',
+          version: '1.0.0',
+          installMetadata,
+          path: extensionDir,
+          // Mocks for other required props
+          contextFiles: [],
+          mcpServers: {},
+          hooks: undefined,
+          isActive: true,
+          id: 'test-id',
+          settings: [],
+          resolvedSettings: [],
+          skills: [],
+        } as unknown as GeminiCLIExtension,
+      ]);
+      vi.spyOn(manager, 'uninstallExtension').mockResolvedValue(undefined);
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      vi.spyOn(manager as any, 'loadExtension').mockResolvedValue(
+        {} as unknown as GeminiCLIExtension,
+      );
+      vi.spyOn(manager, 'enableExtension').mockResolvedValue(undefined);
+
+      // Mock fs.promises for the operations inside installOrUpdateExtension
+      vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined);
+      vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined);
+      vi.spyOn(fs.promises, 'rm').mockResolvedValue(undefined);
+      vi.mocked(fs.existsSync).mockReturnValue(false); // No hooks
+
+      // Mock copyExtension? It's imported.
+      // We can rely on ignoring the failure if we mock enough.
+      // Actually copyExtension is called. We need to mock it if it does real IO.
+      // But we can just let it fail or mock fs.cp if it uses it.
+      // Let's assume the other mocks cover the critical path to the warning.
+      // Warning happens BEFORE copyExtension?
+      // No, warning is after copyExtension usually.
+      // But in my code:
+      // const missingSettings = await getMissingSettings(...)
+      // if (missingSettings.length > 0) debugLogger.warn(...)
+      // ...
+      // copyExtension(...)
+
+      // Wait, let's check extension-manager.ts order.
+      // Line 303: getMissingSettings
+      // Line 317: if (installMetadata.type === 'local' ...) copyExtension
+
+      // So getMissingSettings is called BEFORE copyExtension.
+
+      try {
+        await manager.installOrUpdateExtension(installMetadata, previousConfig);
+      } catch (_) {
+        // Ignore errors from copyExtension or others, we just want to verify the warning
+      }
+
+      expect(debugLogger.warn).toHaveBeenCalledWith(
+        expect.stringContaining(
+          'Extension "test-ext" has missing settings: s1',
+        ),
+      );
+      expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
+        'warning',
+        expect.stringContaining(
+          'Please run "gemini extensions settings test-ext <setting-name>"',
+        ),
+      );
+    });
+  });
+});