Update extension enablement logic (#8544)

This commit is contained in:
christine betts
2025-09-16 15:51:46 -04:00
committed by GitHub
parent 88272cba8b
commit 459de383b2
10 changed files with 1521 additions and 1185 deletions

View File

@@ -14,7 +14,7 @@ interface DisableArgs {
scope: SettingScope;
}
export async function handleDisable(args: DisableArgs) {
export function handleDisable(args: DisableArgs) {
try {
disableExtension(args.name, args.scope);
console.log(
@@ -42,8 +42,8 @@ export const disableCommand: CommandModule = {
choices: [SettingScope.User, SettingScope.Workspace],
})
.check((_argv) => true),
handler: async (argv) => {
await handleDisable({
handler: (argv) => {
handleDisable({
name: argv['name'] as string,
scope: argv['scope'] as SettingScope,
});

View File

@@ -14,12 +14,10 @@ interface EnableArgs {
scope?: SettingScope;
}
export async function handleEnable(args: EnableArgs) {
export function handleEnable(args: EnableArgs) {
try {
const scopes = args.scope
? [args.scope]
: [SettingScope.User, SettingScope.Workspace];
enableExtension(args.name, scopes);
const scope = args.scope ? args.scope : SettingScope.User;
enableExtension(args.name, scope);
if (args.scope) {
console.log(
`Extension "${args.name}" successfully enabled for scope "${args.scope}".`,
@@ -50,8 +48,8 @@ export const enableCommand: CommandModule = {
choices: [SettingScope.User, SettingScope.Workspace],
})
.check((_argv) => true),
handler: async (argv) => {
await handleEnable({
handler: (argv) => {
handleEnable({
name: argv['name'] as string,
scope: argv['scope'] as SettingScope,
});

File diff suppressed because it is too large Load Diff

View File

@@ -27,6 +27,7 @@ import { isWorkspaceTrusted } from './trustedFolders.js';
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
import { randomUUID } from 'node:crypto';
import { ExtensionUpdateState } from '../ui/state/extensions.js';
import { ExtensionEnablementManager } from './extensions/extensionEnablement.js';
export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions');
@@ -140,7 +141,6 @@ export function loadExtensions(
workspaceDir: string = process.cwd(),
): Extension[] {
const settings = loadSettings(workspaceDir).merged;
const disabledExtensions = settings.extensions?.disabled ?? [];
const allExtensions = [...loadUserExtensions()];
if (
@@ -152,10 +152,14 @@ export function loadExtensions(
}
const uniqueExtensions = new Map<string, Extension>();
const manager = new ExtensionEnablementManager(
ExtensionStorage.getUserExtensionsDir(),
);
for (const extension of allExtensions) {
if (
!uniqueExtensions.has(extension.config.name) &&
!disabledExtensions.includes(extension.config.name)
manager.isEnabled(extension.config.name, workspaceDir)
) {
uniqueExtensions.set(extension.config.name, extension);
}
@@ -198,9 +202,6 @@ export function loadExtensionsFromDir(dir: string): Extension[] {
export function loadExtension(extensionDir: string): Extension | null {
if (!fs.statSync(extensionDir).isDirectory()) {
console.error(
`Warning: unexpected file ${extensionDir} in extensions directory.`,
);
return null;
}
@@ -284,7 +285,7 @@ function getContextFileNames(config: ExtensionConfig): string[] {
/**
* Returns an annotated list of extensions. If an extension is listed in enabledExtensionNames, it will be active.
* If enabledExtensionNames is empty, an extension is active unless it is in list of disabled extensions in settings.
* If enabledExtensionNames is empty, an extension is active unless it is disabled.
* @param extensions The base list of extensions.
* @param enabledExtensionNames The names of explicitly enabled extensions.
* @param workspaceDir The current workspace directory.
@@ -294,16 +295,16 @@ export function annotateActiveExtensions(
enabledExtensionNames: string[],
workspaceDir: string,
): GeminiCLIExtension[] {
const settings = loadSettings(workspaceDir).merged;
const disabledExtensions = settings.extensions?.disabled ?? [];
const manager = new ExtensionEnablementManager(
ExtensionStorage.getUserExtensionsDir(),
);
const annotatedExtensions: GeminiCLIExtension[] = [];
if (enabledExtensionNames.length === 0) {
return extensions.map((extension) => ({
name: extension.config.name,
version: extension.config.version,
isActive: !disabledExtensions.includes(extension.config.name),
isActive: manager.isEnabled(extension.config.name, workspaceDir),
path: extension.path,
source: extension.installMetadata?.source,
type: extension.installMetadata?.type,
@@ -526,6 +527,7 @@ export async function installExtension(
),
);
enableExtension(newExtensionConfig!.name, SettingScope.User);
return newExtensionConfig!.name;
} catch (error) {
// Attempt to load config from the source path even if installation fails
@@ -581,11 +583,10 @@ export async function uninstallExtension(
) {
throw new Error(`Extension "${extensionName}" not found.`);
}
removeFromDisabledExtensions(
extensionName,
[SettingScope.User, SettingScope.Workspace],
cwd,
const manager = new ExtensionEnablementManager(
ExtensionStorage.getUserExtensionsDir(),
);
manager.remove(extensionName);
const storage = new ExtensionStorage(extensionName);
await fs.promises.rm(storage.getExtensionDir(), {
@@ -710,45 +711,27 @@ export function disableExtension(
if (scope === SettingScope.System || scope === SettingScope.SystemDefaults) {
throw new Error('System and SystemDefaults scopes are not supported.');
}
const settings = loadSettings(cwd);
const settingsFile = settings.forScope(scope);
const extensionSettings = settingsFile.settings.extensions || {
disabled: [],
};
const disabledExtensions = extensionSettings.disabled || [];
if (!disabledExtensions.includes(name)) {
disabledExtensions.push(name);
extensionSettings.disabled = disabledExtensions;
settings.setValue(scope, 'extensions', extensionSettings);
}
const manager = new ExtensionEnablementManager(
ExtensionStorage.getUserExtensionsDir(),
);
const scopePath = scope === SettingScope.Workspace ? cwd : os.homedir();
manager.disable(name, true, scopePath);
}
export function enableExtension(name: string, scopes: SettingScope[]) {
removeFromDisabledExtensions(name, scopes);
}
/**
* Removes an extension from the list of disabled extensions.
* @param name The name of the extension to remove.
* @param scope The scopes to remove the name from.
*/
function removeFromDisabledExtensions(
export function enableExtension(
name: string,
scopes: SettingScope[],
scope: SettingScope,
cwd: string = process.cwd(),
) {
const settings = loadSettings(cwd);
for (const scope of scopes) {
const settingsFile = settings.forScope(scope);
const extensionSettings = settingsFile.settings.extensions || {
disabled: [],
};
const disabledExtensions = extensionSettings.disabled || [];
extensionSettings.disabled = disabledExtensions.filter(
(extension) => extension !== name,
);
settings.setValue(scope, 'extensions', extensionSettings);
if (scope === SettingScope.System || scope === SettingScope.SystemDefaults) {
throw new Error('System and SystemDefaults scopes are not supported.');
}
const manager = new ExtensionEnablementManager(
ExtensionStorage.getUserExtensionsDir(),
);
const scopePath = scope === SettingScope.Workspace ? cwd : os.homedir();
manager.enable(name, true, scopePath);
}
export async function updateAllUpdatableExtensions(

View File

@@ -0,0 +1,142 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { ExtensionEnablementManager } from './extensionEnablement.js';
// Helper to create a temporary directory for testing
function createTestDir() {
const dirPath = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-test-'));
return {
path: dirPath,
cleanup: () => fs.rmSync(dirPath, { recursive: true, force: true }),
};
}
let testDir: { path: string; cleanup: () => void };
let configDir: string;
let manager: ExtensionEnablementManager;
describe('ExtensionEnablementManager', () => {
beforeEach(() => {
testDir = createTestDir();
configDir = path.join(testDir.path, '.gemini');
manager = new ExtensionEnablementManager(configDir);
});
afterEach(() => {
testDir.cleanup();
// Reset the singleton instance for test isolation
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(ExtensionEnablementManager as any).instance = undefined;
});
describe('isEnabled', () => {
it('should return true if extension is not configured', () => {
expect(manager.isEnabled('ext-test', '/any/path')).toBe(true);
});
it('should return true if no overrides match', () => {
manager.disable('ext-test', false, '/another/path');
expect(manager.isEnabled('ext-test', '/any/path')).toBe(true);
});
it('should enable a path based on an override rule', () => {
manager.disable('ext-test', true, '*'); // Disable globally
manager.enable('ext-test', true, '/home/user/projects/');
expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe(
true,
);
});
it('should disable a path based on a disable override rule', () => {
manager.enable('ext-test', true, '*'); // Enable globally
manager.disable('ext-test', true, '/home/user/projects/');
expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe(
false,
);
});
it('should respect the last matching rule (enable wins)', () => {
manager.disable('ext-test', true, '/home/user/projects/');
manager.enable('ext-test', false, '/home/user/projects/my-app');
expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe(
true,
);
});
it('should respect the last matching rule (disable wins)', () => {
manager.enable('ext-test', true, '/home/user/projects/');
manager.disable('ext-test', false, '/home/user/projects/my-app');
expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe(
false,
);
});
});
describe('includeSubdirs', () => {
it('should add a glob when enabling with includeSubdirs', () => {
manager.enable('ext-test', true, '/path/to/dir');
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('/path/to/dir*');
});
it('should not add a glob when enabling without includeSubdirs', () => {
manager.enable('ext-test', false, '/path/to/dir');
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('/path/to/dir');
expect(config['ext-test'].overrides).not.toContain('/path/to/dir*');
});
it('should add a glob when disabling with includeSubdirs', () => {
manager.disable('ext-test', true, '/path/to/dir');
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('!/path/to/dir*');
});
it('should remove conflicting glob rule when enabling without subdirs', () => {
manager.enable('ext-test', true, '/path/to/dir'); // Adds /path/to/dir*
manager.enable('ext-test', false, '/path/to/dir'); // Should remove the glob
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('/path/to/dir');
expect(config['ext-test'].overrides).not.toContain('/path/to/dir*');
});
it('should remove conflicting non-glob rule when enabling with subdirs', () => {
manager.enable('ext-test', false, '/path/to/dir'); // Adds /path/to/dir
manager.enable('ext-test', true, '/path/to/dir'); // Should remove the non-glob
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('/path/to/dir*');
expect(config['ext-test'].overrides).not.toContain('/path/to/dir');
});
it('should remove conflicting rules when disabling', () => {
manager.enable('ext-test', true, '/path/to/dir'); // enabled with glob
manager.disable('ext-test', false, '/path/to/dir'); // disabled without
const config = manager.readConfig();
expect(config['ext-test'].overrides).toContain('!/path/to/dir');
expect(config['ext-test'].overrides).not.toContain('/path/to/dir*');
});
it('should correctly evaluate isEnabled with subdirs', () => {
manager.disable('ext-test', true, '*');
manager.enable('ext-test', true, '/path/to/dir');
expect(manager.isEnabled('ext-test', '/path/to/dir')).toBe(true);
expect(manager.isEnabled('ext-test', '/path/to/dir/sub')).toBe(true);
expect(manager.isEnabled('ext-test', '/path/to/another')).toBe(false);
});
it('should correctly evaluate isEnabled without subdirs', () => {
manager.disable('ext-test', true, '*');
manager.enable('ext-test', false, '/path/to/dir');
expect(manager.isEnabled('ext-test', '/path/to/dir')).toBe(true);
expect(manager.isEnabled('ext-test', '/path/to/dir/sub')).toBe(false);
});
});
});

View File

@@ -0,0 +1,158 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import fs from 'node:fs';
import path from 'node:path';
export interface ExtensionEnablementConfig {
overrides: string[];
}
export interface AllExtensionsEnablementConfig {
[extensionName: string]: ExtensionEnablementConfig;
}
/**
* Converts a glob pattern to a RegExp object.
* This is a simplified implementation that supports `*`.
*
* @param glob The glob pattern to convert.
* @returns A RegExp object.
*/
function globToRegex(glob: string): RegExp {
const regexString = glob
.replace(/[.+?^${}()|[\]\\]/g, '\\$&') // Escape special regex characters
.replace(/\*/g, '.*'); // Convert * to .*
return new RegExp(`^${regexString}$`);
}
/**
* Determines if an extension is enabled based on the configuration and current path.
* The last matching rule in the overrides list wins.
*
* @param config The enablement configuration for a single extension.
* @param currentPath The absolute path of the current working directory.
* @returns True if the extension is enabled, false otherwise.
*/
export class ExtensionEnablementManager {
private configFilePath: string;
private configDir: string;
constructor(configDir: string) {
this.configDir = configDir;
this.configFilePath = path.join(configDir, 'extension-enablement.json');
}
isEnabled(extensionName: string, currentPath: string): boolean {
const config = this.readConfig();
const extensionConfig = config[extensionName];
// Extensions are enabled by default.
let enabled = true;
for (const rule of extensionConfig?.overrides ?? []) {
const isDisableRule = rule.startsWith('!');
const globPattern = isDisableRule ? rule.substring(1) : rule;
const regex = globToRegex(globPattern);
if (regex.test(currentPath)) {
enabled = !isDisableRule;
}
}
return enabled;
}
readConfig(): AllExtensionsEnablementConfig {
try {
const content = fs.readFileSync(this.configFilePath, 'utf-8');
return JSON.parse(content);
} catch (error) {
if (
error instanceof Error &&
'code' in error &&
error.code === 'ENOENT'
) {
return {};
}
console.error('Error reading extension enablement config:', error);
return {};
}
}
writeConfig(config: AllExtensionsEnablementConfig): void {
fs.mkdirSync(this.configDir, { recursive: true });
fs.writeFileSync(this.configFilePath, JSON.stringify(config, null, 2));
}
enable(
extensionName: string,
includeSubdirs: boolean,
scopePath: string,
): void {
const config = this.readConfig();
if (!config[extensionName]) {
config[extensionName] = { overrides: [] };
}
const pathWithGlob = `${scopePath}*`;
const pathWithoutGlob = scopePath;
const newPath = includeSubdirs ? pathWithGlob : pathWithoutGlob;
const conflictingPath = includeSubdirs ? pathWithoutGlob : pathWithGlob;
config[extensionName].overrides = config[extensionName].overrides.filter(
(rule) =>
rule !== conflictingPath &&
rule !== `!${conflictingPath}` &&
rule !== `!${newPath}`,
);
if (!config[extensionName].overrides.includes(newPath)) {
config[extensionName].overrides.push(newPath);
}
this.writeConfig(config);
}
disable(
extensionName: string,
includeSubdirs: boolean,
scopePath: string,
): void {
const config = this.readConfig();
if (!config[extensionName]) {
config[extensionName] = { overrides: [] };
}
const pathWithGlob = `${scopePath}*`;
const pathWithoutGlob = scopePath;
const targetPath = includeSubdirs ? pathWithGlob : pathWithoutGlob;
const newRule = `!${targetPath}`;
const conflictingPath = includeSubdirs ? pathWithoutGlob : pathWithGlob;
config[extensionName].overrides = config[extensionName].overrides.filter(
(rule) =>
rule !== conflictingPath &&
rule !== `!${conflictingPath}` &&
rule !== targetPath,
);
if (!config[extensionName].overrides.includes(newRule)) {
config[extensionName].overrides.push(newRule);
}
this.writeConfig(config);
}
remove(extensionName: string): void {
const config = this.readConfig();
if (config[extensionName]) {
delete config[extensionName];
this.writeConfig(config);
}
}
}

View File

@@ -49,6 +49,7 @@ import {
import * as fs from 'node:fs'; // fs will be mocked separately
import stripJsonComments from 'strip-json-comments'; // Will be mocked separately
import { isWorkspaceTrusted } from './trustedFolders.js';
import { disableExtension } from './extension.js';
// These imports will get the versions from the vi.mock('./settings.js', ...) factory.
import {
@@ -61,6 +62,8 @@ import {
needsMigration,
type Settings,
loadEnvironment,
migrateDeprecatedSettings,
SettingScope,
} from './settings.js';
import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core';
@@ -90,6 +93,10 @@ vi.mock('fs', async (importOriginal) => {
};
});
vi.mock('./extension.js', () => ({
disableExtension: vi.fn(),
}));
vi.mock('strip-json-comments', () => ({
default: vi.fn((content) => content),
}));
@@ -2311,4 +2318,122 @@ describe('Settings Loading and Merging', () => {
expect(needsMigration(settings)).toBe(false);
});
});
describe('migrateDeprecatedSettings', () => {
let mockFsExistsSync: Mocked<typeof fs.existsSync>;
let mockFsReadFileSync: Mocked<typeof fs.readFileSync>;
let mockDisableExtension: Mocked<typeof disableExtension>;
beforeEach(() => {
vi.resetAllMocks();
mockFsExistsSync = vi.mocked(fs.existsSync);
mockFsReadFileSync = vi.mocked(fs.readFileSync);
mockDisableExtension = vi.mocked(disableExtension);
(mockFsExistsSync as Mock).mockReturnValue(true);
vi.mocked(isWorkspaceTrusted).mockReturnValue(true);
});
afterEach(() => {
vi.restoreAllMocks();
});
it('should migrate disabled extensions from user and workspace settings', () => {
const userSettingsContent = {
extensions: {
disabled: ['user-ext-1', 'shared-ext'],
},
};
const workspaceSettingsContent = {
extensions: {
disabled: ['workspace-ext-1', 'shared-ext'],
},
};
(mockFsReadFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
);
const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR);
const setValueSpy = vi.spyOn(loadedSettings, 'setValue');
migrateDeprecatedSettings(loadedSettings, MOCK_WORKSPACE_DIR);
// Check user settings migration
expect(mockDisableExtension).toHaveBeenCalledWith(
'user-ext-1',
SettingScope.User,
MOCK_WORKSPACE_DIR,
);
expect(mockDisableExtension).toHaveBeenCalledWith(
'shared-ext',
SettingScope.User,
MOCK_WORKSPACE_DIR,
);
// Check workspace settings migration
expect(mockDisableExtension).toHaveBeenCalledWith(
'workspace-ext-1',
SettingScope.Workspace,
MOCK_WORKSPACE_DIR,
);
expect(mockDisableExtension).toHaveBeenCalledWith(
'shared-ext',
SettingScope.Workspace,
MOCK_WORKSPACE_DIR,
);
// Check that setValue was called to remove the deprecated setting
expect(setValueSpy).toHaveBeenCalledWith(
SettingScope.User,
'extensions',
{
disabled: undefined,
},
);
expect(setValueSpy).toHaveBeenCalledWith(
SettingScope.Workspace,
'extensions',
{
disabled: undefined,
},
);
});
it('should not do anything if there are no deprecated settings', () => {
const userSettingsContent = {
extensions: {
enabled: ['user-ext-1'],
},
};
const workspaceSettingsContent = {
someOtherSetting: 'value',
};
(mockFsReadFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
);
const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR);
const setValueSpy = vi.spyOn(loadedSettings, 'setValue');
migrateDeprecatedSettings(loadedSettings, MOCK_WORKSPACE_DIR);
expect(mockDisableExtension).not.toHaveBeenCalled();
expect(setValueSpy).not.toHaveBeenCalled();
});
});
});

View File

@@ -30,6 +30,7 @@ import {
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
import { customDeepMerge } from '../utils/deepMerge.js';
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
import { disableExtension } from './extension.js';
function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined {
let current: SettingDefinition | undefined = undefined;
@@ -707,6 +708,31 @@ export function loadSettings(
);
}
export function migrateDeprecatedSettings(
loadedSettings: LoadedSettings,
workspaceDir: string = process.cwd(),
): void {
const processScope = (scope: SettingScope) => {
const settings = loadedSettings.forScope(scope).settings;
if (settings.extensions?.disabled) {
console.log(
`Migrating deprecated extensions.disabled settings from ${scope} settings...`,
);
for (const extension of settings.extensions.disabled ?? []) {
disableExtension(extension, scope, workspaceDir);
}
const newExtensionsValue = { ...settings.extensions };
newExtensionsValue.disabled = undefined;
loadedSettings.setValue(scope, 'extensions', newExtensionsValue);
}
};
processScope(SettingScope.User);
processScope(SettingScope.Workspace);
}
export function saveSettings(settingsFile: SettingsFile): void {
try {
// Ensure the directory exists

View File

@@ -214,6 +214,7 @@ describe('gemini.tsx main function kitty protocol', () => {
ui: {},
},
setValue: vi.fn(),
forScope: () => ({ settings: {}, originalSettings: {}, path: '' }),
} as never);
vi.mocked(parseArguments).mockResolvedValue({
model: undefined,

View File

@@ -15,7 +15,11 @@ import dns from 'node:dns';
import { spawn } from 'node:child_process';
import { start_sandbox } from './utils/sandbox.js';
import type { DnsResolutionOrder, LoadedSettings } from './config/settings.js';
import { loadSettings, SettingScope } from './config/settings.js';
import {
loadSettings,
migrateDeprecatedSettings,
SettingScope,
} from './config/settings.js';
import { themeManager } from './ui/themes/theme-manager.js';
import { getStartupWarnings } from './utils/startupWarnings.js';
import { getUserStartupWarnings } from './utils/userStartupWarnings.js';
@@ -196,7 +200,7 @@ export async function startInteractiveUI(
export async function main() {
setupUnhandledRejectionHandler();
const settings = loadSettings();
migrateDeprecatedSettings(settings);
await cleanupCheckpoints();
const argv = await parseArguments(settings.merged);