From 69da43eb70d946d676e6c7b2d9ea67a0fb356330 Mon Sep 17 00:00:00 2001 From: christine betts Date: Fri, 5 Sep 2025 11:44:41 -0700 Subject: [PATCH] Make 'source' a positional argument in extensions install and set flag to true (#7807) --- .../src/commands/extensions/install.test.ts | 4 +- .../cli/src/commands/extensions/install.ts | 6 +- packages/cli/src/config/config.ts | 2 +- packages/cli/src/config/extension.test.ts | 99 +++++++------------ packages/cli/src/config/extension.ts | 3 +- packages/cli/src/config/settings.ts | 2 +- packages/cli/src/config/settingsSchema.ts | 2 +- .../cli/src/ui/hooks/useWorkspaceMigration.ts | 3 +- packages/core/src/config/config.ts | 4 +- 9 files changed, 52 insertions(+), 73 deletions(-) diff --git a/packages/cli/src/commands/extensions/install.test.ts b/packages/cli/src/commands/extensions/install.test.ts index 707536bd3a..2f66e432a1 100644 --- a/packages/cli/src/commands/extensions/install.test.ts +++ b/packages/cli/src/commands/extensions/install.test.ts @@ -17,14 +17,14 @@ describe('extensions install command', () => { it('should fail if no source is provided', () => { const validationParser = yargs([]).command(installCommand).fail(false); expect(() => validationParser.parse('install')).toThrow( - 'Either --source or --path must be provided.', + 'Either source or --path must be provided.', ); }); it('should fail if both git source and local path are provided', () => { const validationParser = yargs([]).command(installCommand).fail(false); expect(() => - validationParser.parse('install --source some-url --path /some/path'), + validationParser.parse('install some-url --path /some/path'), ).toThrow('Arguments source and path are mutually exclusive'); }); }); diff --git a/packages/cli/src/commands/extensions/install.ts b/packages/cli/src/commands/extensions/install.ts index 4823f5848e..56eb2a1867 100644 --- a/packages/cli/src/commands/extensions/install.ts +++ b/packages/cli/src/commands/extensions/install.ts @@ -65,12 +65,12 @@ export async function handleInstall(args: InstallArgs) { } export const installCommand: CommandModule = { - command: 'install [--source | --path ]', + command: 'install [source]', describe: 'Installs an extension from a git repository (URL or "org/repo") or a local path.', builder: (yargs) => yargs - .option('source', { + .positional('source', { describe: 'The git URL or "org/repo" of the extension to install.', type: 'string', }) @@ -81,7 +81,7 @@ export const installCommand: CommandModule = { .conflicts('source', 'path') .check((argv) => { if (!argv.source && !argv.path) { - throw new Error('Either --source or --path must be provided.'); + throw new Error('Either source or --path must be provided.'); } return true; }), diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index c127ebc98b..3b187f3f56 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -305,7 +305,7 @@ export async function parseArguments(settings: Settings): Promise { // Register MCP subcommands .command(mcpCommand); - if (settings?.experimental?.extensionManagement ?? false) { + if (settings?.experimental?.extensionManagement ?? true) { yargsInstance.command(extensionsCommand); } diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 144fa1cd32..4483a02d2c 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -63,59 +63,36 @@ vi.mock('child_process', async (importOriginal) => { const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); describe('loadExtensions', () => { - let tempWorkspaceDir: string; let tempHomeDir: string; - let workspaceExtensionsDir: string; + let userExtensionsDir: string; beforeEach(() => { - tempWorkspaceDir = fs.mkdtempSync( - path.join(os.tmpdir(), 'gemini-cli-test-workspace-'), - ); tempHomeDir = fs.mkdtempSync( path.join(os.tmpdir(), 'gemini-cli-test-home-'), ); vi.mocked(os.homedir).mockReturnValue(tempHomeDir); vi.mocked(isWorkspaceTrusted).mockReturnValue(true); - workspaceExtensionsDir = path.join( - tempWorkspaceDir, - EXTENSIONS_DIRECTORY_NAME, - ); - fs.mkdirSync(workspaceExtensionsDir, { recursive: true }); + userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME); + fs.mkdirSync(userExtensionsDir, { recursive: true }); }); afterEach(() => { - fs.rmSync(tempWorkspaceDir, { recursive: true, force: true }); fs.rmSync(tempHomeDir, { recursive: true, force: true }); vi.restoreAllMocks(); }); - it('ignores extensions in untrusted workspaces', () => { - vi.mocked(isWorkspaceTrusted).mockReturnValue(false); - - const extensionDir = path.join(workspaceExtensionsDir, 'test-extension'); - fs.mkdirSync(extensionDir, { recursive: true }); - createExtension({ - extensionsDir: workspaceExtensionsDir, - name: 'ext1', - version: '1.0.0', - addContextFile: true, - }); - const extensions = loadExtensions(tempWorkspaceDir); - expect(extensions.length).toBe(0); - }); - it('should include extension path in loaded extension', () => { - const extensionDir = path.join(workspaceExtensionsDir, 'test-extension'); + const extensionDir = path.join(userExtensionsDir, 'test-extension'); fs.mkdirSync(extensionDir, { recursive: true }); createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'test-extension', version: '1.0.0', }); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); expect(extensions[0].path).toBe(extensionDir); expect(extensions[0].config.name).toBe('test-extension'); @@ -123,70 +100,70 @@ describe('loadExtensions', () => { it('should load context file path when GEMINI.md is present', () => { createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'ext1', version: '1.0.0', addContextFile: true, }); createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'ext2', version: '2.0.0', }); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(2); const ext1 = extensions.find((e) => e.config.name === 'ext1'); const ext2 = extensions.find((e) => e.config.name === 'ext2'); expect(ext1?.contextFiles).toEqual([ - path.join(workspaceExtensionsDir, 'ext1', 'GEMINI.md'), + path.join(userExtensionsDir, 'ext1', 'GEMINI.md'), ]); expect(ext2?.contextFiles).toEqual([]); }); it('should load context file path from the extension config', () => { createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'ext1', version: '1.0.0', addContextFile: false, contextFileName: 'my-context-file.md', }); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); const ext1 = extensions.find((e) => e.config.name === 'ext1'); expect(ext1?.contextFiles).toEqual([ - path.join(workspaceExtensionsDir, 'ext1', 'my-context-file.md'), + path.join(userExtensionsDir, 'ext1', 'my-context-file.md'), ]); }); it('should filter out disabled extensions', () => { createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'ext1', version: '1.0.0', }); createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'ext2', version: '2.0.0', }); - const settingsDir = path.join(tempWorkspaceDir, GEMINI_DIR); + const settingsDir = path.join(tempHomeDir, GEMINI_DIR); fs.mkdirSync(settingsDir, { recursive: true }); fs.writeFileSync( path.join(settingsDir, 'settings.json'), JSON.stringify({ extensions: { disabled: ['ext1'] } }), ); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); const activeExtensions = annotateActiveExtensions( extensions, [], - tempWorkspaceDir, + tempHomeDir, ).filter((e) => e.isActive); expect(activeExtensions).toHaveLength(1); expect(activeExtensions[0].name).toBe('ext2'); @@ -194,7 +171,7 @@ describe('loadExtensions', () => { it('should hydrate variables', () => { createExtension({ - extensionsDir: workspaceExtensionsDir, + extensionsDir: userExtensionsDir, name: 'test-extension', version: '1.0.0', addContextFile: false, @@ -206,11 +183,11 @@ describe('loadExtensions', () => { }, }); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); const loadedConfig = extensions[0].config; const expectedCwd = path.join( - workspaceExtensionsDir, + userExtensionsDir, 'test-extension', 'server', ); @@ -218,6 +195,9 @@ describe('loadExtensions', () => { }); it('should load a linked extension correctly', async () => { + const tempWorkspaceDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-workspace-'), + ); const sourceExtDir = createExtension({ extensionsDir: tempWorkspaceDir, name: 'my-linked-extension', @@ -231,7 +211,7 @@ describe('loadExtensions', () => { type: 'link', }); expect(extensionName).toEqual('my-linked-extension'); - const extensions = loadExtensions(tempHomeDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); const linkedExt = extensions[0]; @@ -252,13 +232,13 @@ describe('loadExtensions', () => { process.env.TEST_DB_URL = 'postgresql://localhost:5432/testdb'; try { - const workspaceExtensionsDir = path.join( - tempWorkspaceDir, + const userExtensionsDir = path.join( + tempHomeDir, EXTENSIONS_DIRECTORY_NAME, ); - fs.mkdirSync(workspaceExtensionsDir, { recursive: true }); + fs.mkdirSync(userExtensionsDir, { recursive: true }); - const extDir = path.join(workspaceExtensionsDir, 'test-extension'); + const extDir = path.join(userExtensionsDir, 'test-extension'); fs.mkdirSync(extDir); // Write config to a separate file for clarity and good practices @@ -280,7 +260,7 @@ describe('loadExtensions', () => { }; fs.writeFileSync(configPath, JSON.stringify(extensionConfig)); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); const extension = extensions[0]; @@ -302,13 +282,10 @@ describe('loadExtensions', () => { }); it('should handle missing environment variables gracefully', () => { - const workspaceExtensionsDir = path.join( - tempWorkspaceDir, - EXTENSIONS_DIRECTORY_NAME, - ); - fs.mkdirSync(workspaceExtensionsDir, { recursive: true }); + const userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME); + fs.mkdirSync(userExtensionsDir, { recursive: true }); - const extDir = path.join(workspaceExtensionsDir, 'test-extension'); + const extDir = path.join(userExtensionsDir, 'test-extension'); fs.mkdirSync(extDir); const extensionConfig = { @@ -331,7 +308,7 @@ describe('loadExtensions', () => { JSON.stringify(extensionConfig), ); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(1); const extension = extensions[0]; @@ -592,7 +569,7 @@ describe('uninstallExtension', () => { await uninstallExtension('my-local-extension'); expect(fs.existsSync(sourceExtDir)).toBe(false); - expect(loadExtensions(tempHomeDir)).toHaveLength(1); + expect(loadExtensions()).toHaveLength(1); expect(fs.existsSync(otherExtDir)).toBe(true); }); @@ -675,7 +652,7 @@ describe('performWorkspaceExtensionMigration', () => { }); await performWorkspaceExtensionMigration([loadExtension(ext1Path)!]); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toEqual([]); }); @@ -703,7 +680,7 @@ describe('performWorkspaceExtensionMigration', () => { const userExtensionsDir = path.join(tempHomeDir, GEMINI_DIR, 'extensions'); const userExt1Path = path.join(userExtensionsDir, 'ext1'); - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); expect(extensions).toHaveLength(2); const metadataPath = path.join(userExt1Path, INSTALL_METADATA_FILENAME); @@ -912,7 +889,7 @@ describe('enableExtension', () => { }); const getActiveExtensions = (): GeminiCLIExtension[] => { - const extensions = loadExtensions(tempWorkspaceDir); + const extensions = loadExtensions(); const activeExtensions = annotateActiveExtensions( extensions, [], diff --git a/packages/cli/src/config/extension.ts b/packages/cli/src/config/extension.ts index e92bd65699..f1d85aa5c2 100644 --- a/packages/cli/src/config/extension.ts +++ b/packages/cli/src/config/extension.ts @@ -119,7 +119,8 @@ export function loadExtensions( if ( (isWorkspaceTrusted(settings) ?? true) && - !settings.experimental?.extensionManagement + // Default management setting to true + !(settings.experimental?.extensionManagement ?? true) ) { allExtensions.push(...getWorkspaceExtensions(workspaceDir)); } diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 33a9b71856..ab59d9b7f4 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -76,7 +76,7 @@ const MIGRATION_MAP: Record = { excludeTools: 'tools.exclude', excludeMCPServers: 'mcp.excluded', excludedProjectEnvVars: 'advanced.excludedEnvVars', - extensionManagement: 'advanced.extensionManagement', + extensionManagement: 'experimental.extensionManagement', extensions: 'extensions', fileFiltering: 'context.fileFiltering', folderTrustFeature: 'security.folderTrust.featureEnabled', diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 7f607acc69..6795065f59 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -838,7 +838,7 @@ export const SETTINGS_SCHEMA = { label: 'Extension Management', category: 'Experimental', requiresRestart: true, - default: false, + default: true, description: 'Enable extension management features.', showInDialog: false, }, diff --git a/packages/cli/src/ui/hooks/useWorkspaceMigration.ts b/packages/cli/src/ui/hooks/useWorkspaceMigration.ts index 63f4f8f045..8f9a3addaf 100644 --- a/packages/cli/src/ui/hooks/useWorkspaceMigration.ts +++ b/packages/cli/src/ui/hooks/useWorkspaceMigration.ts @@ -20,7 +20,8 @@ export function useWorkspaceMigration(settings: LoadedSettings) { ); useEffect(() => { - if (!settings.merged.experimental?.extensionManagement) { + // Default to true if not set. + if (!(settings.merged.experimental?.extensionManagement ?? true)) { return; } const cwd = process.cwd(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 7adf182f64..9d8861ef95 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -282,7 +282,7 @@ export class Config { private readonly useRipgrep: boolean; private readonly shouldUseNodePtyShell: boolean; private readonly skipNextSpeakerCheck: boolean; - private readonly extensionManagement: boolean; + private readonly extensionManagement: boolean = true; private readonly enablePromptCompletion: boolean = false; private initialized: boolean = false; readonly storage: Storage; @@ -360,7 +360,7 @@ export class Config { this.shouldUseNodePtyShell = params.shouldUseNodePtyShell ?? false; this.skipNextSpeakerCheck = params.skipNextSpeakerCheck ?? false; this.useSmartEdit = params.useSmartEdit ?? true; - this.extensionManagement = params.extensionManagement ?? false; + this.extensionManagement = params.extensionManagement ?? true; this.storage = new Storage(this.targetDir); this.enablePromptCompletion = params.enablePromptCompletion ?? false; this.fileExclusions = new FileExclusions(this);