diff --git a/packages/cli/src/commands/extensions/utils.ts b/packages/cli/src/commands/extensions/utils.ts index 1571c56794..941d86ed77 100644 --- a/packages/cli/src/commands/extensions/utils.ts +++ b/packages/cli/src/commands/extensions/utils.ts @@ -8,7 +8,10 @@ import { ExtensionManager } from '../../config/extension-manager.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { loadSettings } from '../../config/settings.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; -import { debugLogger } from '@google/gemini-cli-core'; +import { + debugLogger, + type ResolvedExtensionSetting, +} from '@google/gemini-cli-core'; export async function getExtensionManager() { const workspaceDir = process.cwd(); @@ -35,3 +38,15 @@ export async function getExtensionAndManager(name: string) { return { extension, extensionManager }; } + +export function getFormattedSettingValue( + setting: ResolvedExtensionSetting, +): string { + if (!setting.value) { + return '[not set]'; + } + if (setting.sensitive) { + return '***'; + } + return setting.value; +} diff --git a/packages/cli/src/config/extension-manager-scope.test.ts b/packages/cli/src/config/extension-manager-scope.test.ts index 5079075366..aeddb8c707 100644 --- a/packages/cli/src/config/extension-manager-scope.test.ts +++ b/packages/cli/src/config/extension-manager-scope.test.ts @@ -195,7 +195,7 @@ describe('ExtensionManager Settings Scope', () => { (s) => s.envVar === 'TEST_SETTING', ); expect(setting).toBeDefined(); - expect(setting?.value).toBe('[not set]'); + expect(setting?.value).toBeUndefined(); expect(setting?.scope).toBeUndefined(); // Verify output string does not contain scope diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 9e19109eda..88edb500fe 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -70,6 +70,7 @@ import { } from './extensions/extensionSettings.js'; import type { EventEmitter } from 'node:stream'; import { themeManager } from '../ui/themes/theme-manager.js'; +import { getFormattedSettingValue } from '../commands/extensions/utils.js'; interface ExtensionManagerParams { enabledExtensionOverrides?: string[]; @@ -648,12 +649,7 @@ Would you like to attempt to install via "git clone" instead?`, resolvedSettings.push({ name: setting.name, envVar: setting.envVar, - value: - value === undefined - ? '[not set]' - : setting.sensitive - ? '***' - : value, + value, sensitive: setting.sensitive ?? false, scope, source, @@ -941,7 +937,7 @@ Would you like to attempt to install via "git clone" instead?`, } scope += ')'; } - output += `\n ${setting.name}: ${setting.value} ${scope}`; + output += `\n ${setting.name}: ${getFormattedSettingValue(setting)} ${scope}`; }); } return output; diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index 09ed586b82..536611af97 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -786,6 +786,23 @@ describe('extensionSettings', () => { expect(await userKeychain.getSecret('VAR2')).toBeNull(); }); + it('should delete a non-sensitive setting if the new value is empty', async () => { + mockRequestSetting.mockResolvedValue(''); + + await updateSetting( + config, + '12345', + 'VAR1', + mockRequestSetting, + ExtensionSettingScope.USER, + tempWorkspaceDir, + ); + + const expectedEnvPath = path.join(extensionDir, '.env'); + const actualContent = await fsPromises.readFile(expectedEnvPath, 'utf-8'); + expect(actualContent).not.toContain('VAR1='); + }); + it('should not throw if deleting a non-existent sensitive setting with empty value', async () => { mockRequestSetting.mockResolvedValue(''); // Ensure it doesn't exist first diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index 4ba7d34b35..471988c11b 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -251,7 +251,11 @@ export async function updateSetting( } const parsedEnv = dotenv.parse(envContent); - parsedEnv[settingToUpdate.envVar] = newValue; + if (!newValue) { + delete parsedEnv[settingToUpdate.envVar]; + } else { + parsedEnv[settingToUpdate.envVar] = newValue; + } // We only want to write back the variables that are not sensitive. const nonSensitiveSettings: Record = {}; diff --git a/packages/cli/src/ui/components/views/ExtensionsList.tsx b/packages/cli/src/ui/components/views/ExtensionsList.tsx index e42449e828..7b9c66d577 100644 --- a/packages/cli/src/ui/components/views/ExtensionsList.tsx +++ b/packages/cli/src/ui/components/views/ExtensionsList.tsx @@ -9,6 +9,7 @@ import { Box, Text } from 'ink'; import { useUIState } from '../../contexts/UIStateContext.js'; import { ExtensionUpdateState } from '../../state/extensions.js'; import { debugLogger, type GeminiCLIExtension } from '@google/gemini-cli-core'; +import { getFormattedSettingValue } from '../../../commands/extensions/utils.js'; interface ExtensionsList { extensions: readonly GeminiCLIExtension[]; @@ -70,7 +71,7 @@ export const ExtensionsList: React.FC = ({ extensions }) => { settings: {ext.resolvedSettings.map((setting) => ( - - {setting.name}: {setting.value} + - {setting.name}: {getFormattedSettingValue(setting)} {setting.scope && ( {' '} diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 7c2d34bfad..2995078677 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -158,7 +158,7 @@ export interface ExtensionSetting { export interface ResolvedExtensionSetting { name: string; envVar: string; - value: string; + value?: string; sensitive: boolean; scope?: 'user' | 'workspace'; source?: string; diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index e4bbd7d756..3fbd4517a6 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -1556,6 +1556,41 @@ describe('mcp-client', () => { expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('ext-value'); }); + it('should exclude extension settings with undefined values from environment', async () => { + const mockedTransport = vi + .spyOn(SdkClientStdioLib, 'StdioClientTransport') + .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport); + + await createTransport( + 'test-server', + { + command: 'test-command', + extension: { + name: 'test-ext', + resolvedSettings: [ + { + envVar: 'GEMINI_CLI_EXT_VAR', + value: undefined, + sensitive: false, + name: 'ext-setting', + }, + ], + version: '', + isActive: false, + path: '', + contextFiles: [], + id: '', + }, + }, + false, + EMPTY_CONFIG, + ); + + const callArgs = mockedTransport.mock.calls[0][0]; + expect(callArgs.env).toBeDefined(); + expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined(); + }); + describe('useGoogleCredentialProvider', () => { beforeEach(() => { // Mock GoogleAuth client diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index e7aa866a09..3773aae5f2 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1948,7 +1948,9 @@ function getExtensionEnvironment( const env: Record = {}; if (extension?.resolvedSettings) { for (const setting of extension.resolvedSettings) { - env[setting.envVar] = setting.value; + if (setting.value) { + env[setting.envVar] = setting.value; + } } } return env;