From 77e226c55fe7e795982843596ad93ac1a5756983 Mon Sep 17 00:00:00 2001 From: christine betts Date: Fri, 9 Jan 2026 12:04:53 -0500 Subject: [PATCH] Show settings source in extensions lists (#16207) --- .../config/extension-manager-scope.test.ts | 196 ++++++++++++++++++ packages/cli/src/config/extension-manager.ts | 56 ++++- .../extensions/extensionSettings.test.ts | 13 +- .../config/extensions/extensionSettings.ts | 33 ++- .../extensions/extensionUpdates.test.ts | 24 ++- .../components/views/ExtensionsList.test.tsx | 13 +- .../ui/components/views/ExtensionsList.tsx | 9 + packages/core/src/config/config.ts | 2 + 8 files changed, 329 insertions(+), 17 deletions(-) create mode 100644 packages/cli/src/config/extension-manager-scope.test.ts diff --git a/packages/cli/src/config/extension-manager-scope.test.ts b/packages/cli/src/config/extension-manager-scope.test.ts new file mode 100644 index 0000000000..a42e198bd0 --- /dev/null +++ b/packages/cli/src/config/extension-manager-scope.test.ts @@ -0,0 +1,196 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { ExtensionManager } from './extension-manager.js'; +import type { Settings } from './settings.js'; + +let currentTempHome = ''; + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + homedir: () => currentTempHome, + debugLogger: { + log: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + }, + }; +}); + +describe('ExtensionManager Settings Scope', () => { + const extensionName = 'test-extension'; + let tempWorkspace: string; + let extensionsDir: string; + let extensionDir: string; + + beforeEach(async () => { + currentTempHome = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-home-'), + ); + tempWorkspace = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-workspace-'), + ); + extensionsDir = path.join(currentTempHome, '.gemini', 'extensions'); + extensionDir = path.join(extensionsDir, extensionName); + + fs.mkdirSync(extensionDir, { recursive: true }); + + // Create gemini-extension.json + const extensionConfig = { + name: extensionName, + version: '1.0.0', + settings: [ + { + name: 'Test Setting', + envVar: 'TEST_SETTING', + description: 'A test setting', + }, + ], + }; + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify(extensionConfig), + ); + + // Create install metadata + const installMetadata = { + source: extensionDir, + type: 'local', + }; + fs.writeFileSync( + path.join(extensionDir, 'install-metadata.json'), + JSON.stringify(installMetadata), + ); + }); + + afterEach(() => { + // Clean up files if needed, or rely on temp dir cleanup + vi.clearAllMocks(); + }); + + it('should prioritize workspace settings over user settings and report correct scope', async () => { + // 1. Set User Setting + const userSettingsPath = path.join(extensionDir, '.env'); + fs.writeFileSync(userSettingsPath, 'TEST_SETTING=user-value'); + + // 2. Set Workspace Setting + const workspaceSettingsPath = path.join(tempWorkspace, '.env'); + fs.writeFileSync(workspaceSettingsPath, 'TEST_SETTING=workspace-value'); + + const extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspace, + requestConsent: async () => true, + requestSetting: async () => '', + settings: { + telemetry: { + enabled: false, + }, + } as Settings, + }); + + const extensions = await extensionManager.loadExtensions(); + const extension = extensions.find((e) => e.name === extensionName); + + expect(extension).toBeDefined(); + + // Verify resolved settings + const setting = extension?.resolvedSettings?.find( + (s) => s.envVar === 'TEST_SETTING', + ); + expect(setting).toBeDefined(); + expect(setting?.value).toBe('workspace-value'); + expect(setting?.scope).toBe('workspace'); + expect(setting?.source).toBe(workspaceSettingsPath); + + // Verify output string contains (Workspace - ) + const output = extensionManager.toOutputString(extension!); + expect(output).toContain( + `Test Setting: workspace-value (Workspace - ${workspaceSettingsPath})`, + ); + }); + + it('should fallback to user settings if workspace setting is missing', async () => { + // 1. Set User Setting + const userSettingsPath = path.join(extensionDir, '.env'); + fs.writeFileSync(userSettingsPath, 'TEST_SETTING=user-value'); + + // 2. No Workspace Setting + + const extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspace, + requestConsent: async () => true, + requestSetting: async () => '', + settings: { + telemetry: { + enabled: false, + }, + } as Settings, + }); + + const extensions = await extensionManager.loadExtensions(); + const extension = extensions.find((e) => e.name === extensionName); + + expect(extension).toBeDefined(); + + // Verify resolved settings + const setting = extension?.resolvedSettings?.find( + (s) => s.envVar === 'TEST_SETTING', + ); + expect(setting).toBeDefined(); + expect(setting?.value).toBe('user-value'); + expect(setting?.scope).toBe('user'); + expect(setting?.source?.endsWith(path.join(extensionName, '.env'))).toBe( + true, + ); + + // Verify output string contains (User - ) + const output = extensionManager.toOutputString(extension!); + expect(output).toContain( + `Test Setting: user-value (User - ${userSettingsPath})`, + ); + }); + + it('should report unset if neither is present', async () => { + // No settings files + + const extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspace, + requestConsent: async () => true, + requestSetting: async () => '', + settings: { + telemetry: { + enabled: false, + }, + } as Settings, + }); + + const extensions = await extensionManager.loadExtensions(); + const extension = extensions.find((e) => e.name === extensionName); + + expect(extension).toBeDefined(); + + // Verify resolved settings + const setting = extension?.resolvedSettings?.find( + (s) => s.envVar === 'TEST_SETTING', + ); + expect(setting).toBeDefined(); + expect(setting?.value).toBe('[not set]'); + expect(setting?.scope).toBeUndefined(); + + // Verify output string does not contain scope + const output = extensionManager.toOutputString(extension!); + expect(output).toContain('Test Setting: [not set]'); + expect(output).not.toContain('Test Setting: [not set] (User)'); + expect(output).not.toContain('Test Setting: [not set] (Workspace)'); + }); +}); diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 998b91529c..d979692441 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -59,9 +59,12 @@ import { } from './extensions/variables.js'; import { getEnvContents, + getEnvFilePath, maybePromptForSettings, getMissingSettings, type ExtensionSetting, + getScopedEnvContents, + ExtensionSettingScope, } from './extensions/extensionSettings.js'; import type { EventEmitter } from 'node:stream'; import { getEnableHooks } from './settingsSchema.js'; @@ -277,6 +280,7 @@ Would you like to attempt to install via "git clone" instead?`, previousSettings = await getEnvContents( previousExtensionConfig, extensionId, + this.workspaceDir, ); await this.uninstallExtension(newExtensionName, isUpdate); } @@ -303,6 +307,7 @@ Would you like to attempt to install via "git clone" instead?`, const missingSettings = await getMissingSettings( newExtensionConfig, extensionId, + this.workspaceDir, ); if (missingSettings.length > 0) { const message = `Extension "${newExtensionConfig.name}" has missing settings: ${missingSettings @@ -518,16 +523,51 @@ Would you like to attempt to install via "git clone" instead?`, ); } - const customEnv = await getEnvContents( + const extensionId = getExtensionId(config, installMetadata); + + const userSettings = await getScopedEnvContents( config, - getExtensionId(config, installMetadata), + extensionId, + ExtensionSettingScope.USER, ); + const workspaceSettings = await getScopedEnvContents( + config, + extensionId, + ExtensionSettingScope.WORKSPACE, + this.workspaceDir, + ); + + const customEnv = { ...userSettings, ...workspaceSettings }; config = resolveEnvVarsInObject(config, customEnv); const resolvedSettings: ResolvedExtensionSetting[] = []; if (config.settings) { for (const setting of config.settings) { const value = customEnv[setting.envVar]; + let scope: 'user' | 'workspace' | undefined; + let source: string | undefined; + + // Note: strict check for undefined, as empty string is a valid value + if (workspaceSettings[setting.envVar] !== undefined) { + scope = 'workspace'; + if (setting.sensitive) { + source = 'Keychain'; + } else { + source = getEnvFilePath( + config.name, + ExtensionSettingScope.WORKSPACE, + this.workspaceDir, + ); + } + } else if (userSettings[setting.envVar] !== undefined) { + scope = 'user'; + if (setting.sensitive) { + source = 'Keychain'; + } else { + source = getEnvFilePath(config.name, ExtensionSettingScope.USER); + } + } + resolvedSettings.push({ name: setting.name, envVar: setting.envVar, @@ -538,6 +578,8 @@ Would you like to attempt to install via "git clone" instead?`, ? '***' : value, sensitive: setting.sensitive ?? false, + scope, + source, }); } } @@ -754,7 +796,15 @@ Would you like to attempt to install via "git clone" instead?`, if (resolvedSettings && resolvedSettings.length > 0) { output += `\n Settings:`; resolvedSettings.forEach((setting) => { - output += `\n ${setting.name}: ${setting.value}`; + let scope = ''; + if (setting.scope) { + scope = setting.scope === 'workspace' ? '(Workspace' : '(User'; + if (setting.source) { + scope += ` - ${setting.source}`; + } + scope += ')'; + } + output += `\n ${setting.name}: ${setting.value} ${scope}`; }); } return output; diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index 39b1eafe40..db527f1ecb 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -529,6 +529,7 @@ describe('extensionSettings', () => { config, extensionId, ExtensionSettingScope.USER, + tempWorkspaceDir, ); expect(contents).toEqual({ @@ -552,6 +553,7 @@ describe('extensionSettings', () => { config, extensionId, ExtensionSettingScope.WORKSPACE, + tempWorkspaceDir, ); expect(contents).toEqual({ @@ -596,7 +598,11 @@ describe('extensionSettings', () => { ); await workspaceKeychain.setSecret('VAR2', 'workspace-secret2'); - const contents = await getEnvContents(config, extensionId); + const contents = await getEnvContents( + config, + extensionId, + tempWorkspaceDir, + ); expect(contents).toEqual({ VAR1: 'workspace-value1', @@ -636,6 +642,7 @@ describe('extensionSettings', () => { 'VAR1', mockRequestSetting, ExtensionSettingScope.USER, + tempWorkspaceDir, ); const expectedEnvPath = path.join(extensionDir, '.env'); @@ -652,6 +659,7 @@ describe('extensionSettings', () => { 'VAR1', mockRequestSetting, ExtensionSettingScope.WORKSPACE, + tempWorkspaceDir, ); const expectedEnvPath = path.join(tempWorkspaceDir, '.env'); @@ -668,6 +676,7 @@ describe('extensionSettings', () => { 'VAR2', mockRequestSetting, ExtensionSettingScope.USER, + tempWorkspaceDir, ); const userKeychain = new KeychainTokenStorage( @@ -685,6 +694,7 @@ describe('extensionSettings', () => { 'VAR2', mockRequestSetting, ExtensionSettingScope.WORKSPACE, + tempWorkspaceDir, ); const workspaceKeychain = new KeychainTokenStorage( @@ -710,6 +720,7 @@ describe('extensionSettings', () => { 'VAR1', mockRequestSetting, ExtensionSettingScope.WORKSPACE, + tempWorkspaceDir, ); // Read the .env file after update diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index 582a94c807..482c206cd6 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -33,20 +33,28 @@ const getKeychainStorageName = ( extensionName: string, extensionId: string, scope: ExtensionSettingScope, + workspaceDir?: string, ): string => { const base = `Gemini CLI Extensions ${extensionName} ${extensionId}`; if (scope === ExtensionSettingScope.WORKSPACE) { - return `${base} ${process.cwd()}`; + if (!workspaceDir) { + throw new Error('Workspace directory is required for workspace scope'); + } + return `${base} ${workspaceDir}`; } return base; }; -const getEnvFilePath = ( +export const getEnvFilePath = ( extensionName: string, scope: ExtensionSettingScope, + workspaceDir?: string, ): string => { if (scope === ExtensionSettingScope.WORKSPACE) { - return path.join(process.cwd(), EXTENSION_SETTINGS_FILENAME); + if (!workspaceDir) { + throw new Error('Workspace directory is required for workspace scope'); + } + return path.join(workspaceDir, EXTENSION_SETTINGS_FILENAME); } return new ExtensionStorage(extensionName).getEnvFilePath(); }; @@ -143,12 +151,13 @@ export async function getScopedEnvContents( extensionConfig: ExtensionConfig, extensionId: string, scope: ExtensionSettingScope, + workspaceDir?: string, ): Promise> { const { name: extensionName } = extensionConfig; const keychain = new KeychainTokenStorage( - getKeychainStorageName(extensionName, extensionId, scope), + getKeychainStorageName(extensionName, extensionId, scope, workspaceDir), ); - const envFilePath = getEnvFilePath(extensionName, scope); + const envFilePath = getEnvFilePath(extensionName, scope, workspaceDir); let customEnv: Record = {}; if (fsSync.existsSync(envFilePath)) { const envFile = fsSync.readFileSync(envFilePath, 'utf-8'); @@ -171,6 +180,7 @@ export async function getScopedEnvContents( export async function getEnvContents( extensionConfig: ExtensionConfig, extensionId: string, + workspaceDir: string, ): Promise> { if (!extensionConfig.settings || extensionConfig.settings.length === 0) { return Promise.resolve({}); @@ -185,6 +195,7 @@ export async function getEnvContents( extensionConfig, extensionId, ExtensionSettingScope.WORKSPACE, + workspaceDir, ); return { ...userSettings, ...workspaceSettings }; @@ -196,6 +207,7 @@ export async function updateSetting( settingKey: string, requestSetting: (setting: ExtensionSetting) => Promise, scope: ExtensionSettingScope, + workspaceDir?: string, ): Promise { const { name: extensionName, settings } = extensionConfig; if (!settings || settings.length === 0) { @@ -214,7 +226,7 @@ export async function updateSetting( const newValue = await requestSetting(settingToUpdate); const keychain = new KeychainTokenStorage( - getKeychainStorageName(extensionName, extensionId, scope), + getKeychainStorageName(extensionName, extensionId, scope, workspaceDir), ); if (settingToUpdate.sensitive) { @@ -224,7 +236,7 @@ export async function updateSetting( // For non-sensitive settings, we need to read the existing .env file, // update the value, and write it back, preserving any other values. - const envFilePath = getEnvFilePath(extensionName, scope); + const envFilePath = getEnvFilePath(extensionName, scope, workspaceDir); let envContent = ''; if (fsSync.existsSync(envFilePath)) { envContent = await fs.readFile(envFilePath, 'utf-8'); @@ -302,13 +314,18 @@ async function clearSettings( export async function getMissingSettings( extensionConfig: ExtensionConfig, extensionId: string, + workspaceDir: string, ): Promise { const { settings } = extensionConfig; if (!settings || settings.length === 0) { return []; } - const existingSettings = await getEnvContents(extensionConfig, extensionId); + const existingSettings = await getEnvContents( + extensionConfig, + extensionId, + workspaceDir, + ); const missingSettings: ExtensionSetting[] = []; for (const setting of settings) { diff --git a/packages/cli/src/config/extensions/extensionUpdates.test.ts b/packages/cli/src/config/extensions/extensionUpdates.test.ts index bdd2841ad6..1e30e0b898 100644 --- a/packages/cli/src/config/extensions/extensionUpdates.test.ts +++ b/packages/cli/src/config/extensions/extensionUpdates.test.ts @@ -154,7 +154,11 @@ describe('extensionUpdates', () => { ); await userKeychain.setSecret('VAR2', 'val2'); - const missing = await getMissingSettings(config, extensionId); + const missing = await getMissingSettings( + config, + extensionId, + tempWorkspaceDir, + ); expect(missing).toEqual([]); }); @@ -166,7 +170,11 @@ describe('extensionUpdates', () => { }; const extensionId = '12345'; - const missing = await getMissingSettings(config, extensionId); + const missing = await getMissingSettings( + config, + extensionId, + tempWorkspaceDir, + ); expect(missing).toHaveLength(1); expect(missing[0].name).toBe('s1'); }); @@ -181,7 +189,11 @@ describe('extensionUpdates', () => { }; const extensionId = '12345'; - const missing = await getMissingSettings(config, extensionId); + const missing = await getMissingSettings( + config, + extensionId, + tempWorkspaceDir, + ); expect(missing).toHaveLength(1); expect(missing[0].name).toBe('s2'); }); @@ -201,7 +213,11 @@ describe('extensionUpdates', () => { ); fs.writeFileSync(workspaceEnvPath, 'VAR1=val1'); - const missing = await getMissingSettings(config, extensionId); + const missing = await getMissingSettings( + config, + extensionId, + tempWorkspaceDir, + ); expect(missing).toEqual([]); }); }); diff --git a/packages/cli/src/ui/components/views/ExtensionsList.test.tsx b/packages/cli/src/ui/components/views/ExtensionsList.test.tsx index 7fb5d2f361..b1aa1e83db 100644 --- a/packages/cli/src/ui/components/views/ExtensionsList.test.tsx +++ b/packages/cli/src/ui/components/views/ExtensionsList.test.tsx @@ -142,6 +142,16 @@ describe('', () => { value: '1000', envVar: 'MAX_TOKENS', sensitive: false, + scope: 'user' as const, + source: '/path/to/.env', + }, + { + name: 'model', + value: 'gemini-pro', + envVar: 'MODEL', + sensitive: false, + scope: 'workspace' as const, + source: 'Keychain', }, ], }; @@ -151,7 +161,8 @@ describe('', () => { const output = lastFrame(); expect(output).toContain('settings:'); expect(output).toContain('- sensitiveApiKey: ***'); - expect(output).toContain('- maxTokens: 1000'); + expect(output).toContain('- maxTokens: 1000 (User - /path/to/.env)'); + expect(output).toContain('- model: gemini-pro (Workspace - Keychain)'); unmount(); }); }); diff --git a/packages/cli/src/ui/components/views/ExtensionsList.tsx b/packages/cli/src/ui/components/views/ExtensionsList.tsx index 2d456625f1..27ce6f5e92 100644 --- a/packages/cli/src/ui/components/views/ExtensionsList.tsx +++ b/packages/cli/src/ui/components/views/ExtensionsList.tsx @@ -71,6 +71,15 @@ export const ExtensionsList: React.FC = ({ extensions }) => { {ext.resolvedSettings.map((setting) => ( - {setting.name}: {setting.value} + {setting.scope && ( + + {' '} + ( + {setting.scope.charAt(0).toUpperCase() + + setting.scope.slice(1)} + {setting.source ? ` - ${setting.source}` : ''}) + + )} ))} diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 01615c1081..2a9ae19284 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -149,6 +149,8 @@ export interface ResolvedExtensionSetting { envVar: string; value: string; sensitive: boolean; + scope?: 'user' | 'workspace'; + source?: string; } export interface CliHelpAgentSettings {