Patch #10628 and #10514 into v0.8.0 preview (#10646)

Co-authored-by: Jacob MacDonald <jakemac@google.com>
Co-authored-by: Adam Weidman <65992621+adamfweidman@users.noreply.github.com>

packages/cli/src/commands/extensions/install.test.ts

@@ -10,6 +10,7 @@ import yargs from 'yargs';
 
 const mockInstallExtension = vi.hoisted(() => vi.fn());
 const mockRequestConsentNonInteractive = vi.hoisted(() => vi.fn());
+const mockStat = vi.hoisted(() => vi.fn());
 
 vi.mock('../../config/extension.js', () => ({
   installExtension: mockInstallExtension,
@@ -20,35 +21,20 @@ vi.mock('../../utils/errors.js', () => ({
   getErrorMessage: vi.fn((error: Error) => error.message),
 }));
 
+vi.mock('node:fs/promises', () => ({
+  stat: mockStat,
+  default: {
+    stat: mockStat,
+  },
+}));
+
 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.',
+      'Not enough non-option arguments: got 0, need at least 1',
     );
   });
-
-  it('should fail if both git source and local path are provided', () => {
-    const validationParser = yargs([])
-      .command(installCommand)
-      .fail(false)
-      .locale('en');
-    expect(() =>
-      validationParser.parse('install some-url --path /some/path'),
-    ).toThrow('Arguments source and path are mutually exclusive');
-  });
-
-  it('should fail if both auto update and local path are provided', () => {
-    const validationParser = yargs([])
-      .command(installCommand)
-      .fail(false)
-      .locale('en');
-    expect(() =>
-      validationParser.parse(
-        'install some-url --path /some/path --auto-update',
-      ),
-    ).toThrow('Arguments path and auto-update are mutually exclusive');
-  });
 });
 
 describe('handleInstall', () => {
@@ -67,6 +53,7 @@ describe('handleInstall', () => {
   afterEach(() => {
     mockInstallExtension.mockClear();
     mockRequestConsentNonInteractive.mockClear();
+    mockStat.mockClear();
     vi.resetAllMocks();
   });
 
@@ -107,13 +94,12 @@ describe('handleInstall', () => {
   });
 
   it('throws an error from an unknown source', async () => {
+    mockStat.mockRejectedValue(new Error('ENOENT: no such file or directory'));
     await handleInstall({
       source: 'test://google.com',
     });
 
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      'The source "test://google.com" is not a valid URL format.',
-    );
+    expect(consoleErrorSpy).toHaveBeenCalledWith('Install source not found.');
     expect(processSpy).toHaveBeenCalledWith(1);
   });
 
@@ -131,9 +117,9 @@ describe('handleInstall', () => {
 
   it('should install an extension from a local path', async () => {
     mockInstallExtension.mockResolvedValue('local-extension');
-
+    mockStat.mockResolvedValue({});
     await handleInstall({
-      path: '/some/path',
+      source: '/some/path',
     });
 
     expect(consoleLogSpy).toHaveBeenCalledWith(
@@ -141,15 +127,6 @@ describe('handleInstall', () => {
     );
   });
 
-  it('should throw an error if no source or path is provided', async () => {
-    await handleInstall({});
-
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      'Either --source or --path must be provided.',
-    );
-    expect(processSpy).toHaveBeenCalledWith(1);
-  });
-
   it('should throw an error if install extension fails', async () => {
     mockInstallExtension.mockRejectedValue(
       new Error('Install extension failed'),

packages/cli/src/commands/extensions/install.ts

@@ -10,12 +10,11 @@ import {
   requestConsentNonInteractive,
 } from '../../config/extension.js';
 import type { ExtensionInstallMetadata } from '@google/gemini-cli-core';
-
 import { getErrorMessage } from '../../utils/errors.js';
+import { stat } from 'node:fs/promises';
 
 interface InstallArgs {
-  source?: string;
-  path?: string;
+  source: string;
   ref?: string;
   autoUpdate?: boolean;
 }
@@ -23,32 +22,34 @@ interface InstallArgs {
 export async function handleInstall(args: InstallArgs) {
   try {
     let installMetadata: ExtensionInstallMetadata;
-    if (args.source) {
-      const { source } = args;
-      if (
-        source.startsWith('http://') ||
-        source.startsWith('https://') ||
-        source.startsWith('git@') ||
-        source.startsWith('sso://')
-      ) {
-        installMetadata = {
-          source,
-          type: 'git',
-          ref: args.ref,
-          autoUpdate: args.autoUpdate,
-        };
-      } else {
-        throw new Error(`The source "${source}" is not a valid URL format.`);
-      }
-    } else if (args.path) {
+    const { source } = args;
+    if (
+      source.startsWith('http://') ||
+      source.startsWith('https://') ||
+      source.startsWith('git@') ||
+      source.startsWith('sso://')
+    ) {
       installMetadata = {
-        source: args.path,
-        type: 'local',
+        source,
+        type: 'git',
+        ref: args.ref,
         autoUpdate: args.autoUpdate,
       };
     } else {
-      // This should not be reached due to the yargs check.
-      throw new Error('Either --source or --path must be provided.');
+      if (args.ref || args.autoUpdate) {
+        throw new Error(
+          '--ref and --auto-update are not applicable for local extensions.',
+        );
+      }
+      try {
+        await stat(source);
+        installMetadata = {
+          source,
+          type: 'local',
+        };
+      } catch {
+        throw new Error('Install source not found.');
+      }
     }
 
     const name = await installExtension(
@@ -63,17 +64,14 @@ export async function handleInstall(args: InstallArgs) {
 }
 
 export const installCommand: CommandModule = {
-  command: 'install [<source>] [--path] [--ref] [--auto-update]',
+  command: 'install <source>',
   describe: 'Installs an extension from a git repository URL or a local path.',
   builder: (yargs) =>
     yargs
       .positional('source', {
-        describe: 'The github URL of the extension to install.',
-        type: 'string',
-      })
-      .option('path', {
-        describe: 'Path to a local extension directory.',
+        describe: 'The github URL or local path of the extension to install.',
         type: 'string',
+        demandOption: true,
       })
       .option('ref', {
         describe: 'The git ref to install from.',
@@ -83,19 +81,15 @@ export const installCommand: CommandModule = {
         describe: 'Enable auto-update for this extension.',
         type: 'boolean',
       })
-      .conflicts('source', 'path')
-      .conflicts('path', 'ref')
-      .conflicts('path', 'auto-update')
       .check((argv) => {
-        if (!argv.source && !argv.path) {
-          throw new Error('Either source or --path must be provided.');
+        if (!argv.source) {
+          throw new Error('The source argument must be provided.');
         }
         return true;
       }),
   handler: async (argv) => {
     await handleInstall({
-      source: argv['source'] as string | undefined,
-      path: argv['path'] as string | undefined,
+      source: argv['source'] as string,
       ref: argv['ref'] as string | undefined,
       autoUpdate: argv['auto-update'] as boolean | undefined,
     });

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

@@ -8,12 +8,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import {
   checkForExtensionUpdate,
   cloneFromGit,
+  extractFile,
   findReleaseAsset,
   parseGitHubRepoForReleases,
 } from './github.js';
 import { simpleGit, type SimpleGit } from 'simple-git';
 import { ExtensionUpdateState } from '../../ui/state/extensions.js';
-import type * as os from 'node:os';
+import * as os from 'node:os';
+import * as fs from 'node:fs/promises';
+import * as fsSync from 'node:fs';
+import * as path from 'node:path';
+import * as tar from 'tar';
+import * as archiver from 'archiver';
 import type { GeminiCLIExtension } from '@google/gemini-cli-core';
 
 const mockPlatform = vi.hoisted(() => vi.fn());
@@ -341,4 +347,83 @@ describe('git extension helpers', () => {
       );
     });
   });
+
+  describe('extractFile', () => {
+    let tempDir: string;
+
+    beforeEach(async () => {
+      tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-test-'));
+    });
+
+    afterEach(async () => {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    });
+
+    it('should extract a .tar.gz file', async () => {
+      const archivePath = path.join(tempDir, 'test.tar.gz');
+      const extractionDest = path.join(tempDir, 'extracted');
+      await fs.mkdir(extractionDest);
+
+      // Create a dummy file to be archived
+      const dummyFilePath = path.join(tempDir, 'test.txt');
+      await fs.writeFile(dummyFilePath, 'hello tar');
+
+      // Create the tar.gz file
+      await tar.c(
+        {
+          gzip: true,
+          file: archivePath,
+          cwd: tempDir,
+        },
+        ['test.txt'],
+      );
+
+      await extractFile(archivePath, extractionDest);
+
+      const extractedFilePath = path.join(extractionDest, 'test.txt');
+      const content = await fs.readFile(extractedFilePath, 'utf-8');
+      expect(content).toBe('hello tar');
+    });
+
+    it('should extract a .zip file', async () => {
+      const archivePath = path.join(tempDir, 'test.zip');
+      const extractionDest = path.join(tempDir, 'extracted');
+      await fs.mkdir(extractionDest);
+
+      // Create a dummy file to be archived
+      const dummyFilePath = path.join(tempDir, 'test.txt');
+      await fs.writeFile(dummyFilePath, 'hello zip');
+
+      // Create the zip file
+      const output = fsSync.createWriteStream(archivePath);
+      const archive = archiver.create('zip');
+
+      const streamFinished = new Promise((resolve, reject) => {
+        output.on('close', () => resolve(null));
+        archive.on('error', reject);
+      });
+
+      archive.pipe(output);
+      archive.file(dummyFilePath, { name: 'test.txt' });
+      await archive.finalize();
+      await streamFinished;
+
+      await extractFile(archivePath, extractionDest);
+
+      const extractedFilePath = path.join(extractionDest, 'test.txt');
+      const content = await fs.readFile(extractedFilePath, 'utf-8');
+      expect(content).toBe('hello zip');
+    });
+
+    it('should throw an error for unsupported file types', async () => {
+      const unsupportedFilePath = path.join(tempDir, 'test.txt');
+      await fs.writeFile(unsupportedFilePath, 'some content');
+      const extractionDest = path.join(tempDir, 'extracted');
+      await fs.mkdir(extractionDest);
+
+      await expect(
+        extractFile(unsupportedFilePath, extractionDest),
+      ).rejects.toThrow('Unsupported file extension for extraction:');
+    });
+  });
 });

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

@@ -15,8 +15,9 @@ import * as os from 'node:os';
 import * as https from 'node:https';
 import * as fs from 'node:fs';
 import * as path from 'node:path';
-import { spawnSync } from 'node:child_process';
 import { EXTENSIONS_CONFIG_FILENAME, loadExtension } from '../extension.js';
+import * as tar from 'tar';
+import extract from 'extract-zip';
 
 function getGitHubToken(): string | undefined {
   return process.env['GITHUB_TOKEN'];
@@ -270,7 +271,7 @@ export async function downloadFromGitHubRelease(
 
     await downloadFile(archiveUrl, downloadedAssetPath);
 
-    extractFile(downloadedAssetPath, destination);
+    await extractFile(downloadedAssetPath, destination);
 
     // For regular github releases, the repository is put inside of a top level
     // directory. In this case we should see exactly two file in the destination
@@ -416,27 +417,15 @@ async function downloadFile(url: string, dest: string): Promise<void> {
   });
 }
 
-function extractFile(file: string, dest: string) {
-  let args: string[];
-  let command: 'tar' | 'unzip';
+export async function extractFile(file: string, dest: string): Promise<void> {
   if (file.endsWith('.tar.gz')) {
-    args = ['-xzf', file, '-C', dest];
-    command = 'tar';
+    await tar.x({
+      file,
+      cwd: dest,
+    });
   } else if (file.endsWith('.zip')) {
-    args = [file, '-d', dest];
-    command = 'unzip';
+    await extract(file, { dir: dest });
   } else {
     throw new Error(`Unsupported file extension for extraction: ${file}`);
   }
-
-  const result = spawnSync(command, args, { stdio: 'pipe' });
-
-  if (result.status !== 0) {
-    if (result.error) {
-      throw new Error(`Failed to spawn '${command}': ${result.error.message}`);
-    }
-    throw new Error(
-      `'${command}' command failed with exit code ${result.status}: ${result.stderr.toString()}`,
-    );
-  }
 }

packages/cli/src/ui/components/FolderTrustDialog.test.tsx

@@ -17,8 +17,9 @@ vi.mock('../../utils/processUtils.js', () => ({
 const mockedExit = vi.hoisted(() => vi.fn());
 const mockedCwd = vi.hoisted(() => vi.fn());
 
-vi.mock('process', async () => {
-  const actual = await vi.importActual('process');
+vi.mock('node:process', async () => {
+  const actual =
+    await vi.importActual<typeof import('node:process')>('node:process');
   return {
     ...actual,
     exit: mockedExit,

packages/cli/src/ui/hooks/useFolderTrust.test.ts

@@ -11,14 +11,19 @@ import type { LoadedSettings } from '../../config/settings.js';
 import { FolderTrustChoice } from '../components/FolderTrustDialog.js';
 import type { LoadedTrustedFolders } from '../../config/trustedFolders.js';
 import { TrustLevel } from '../../config/trustedFolders.js';
-import * as process from 'node:process';
-
 import * as trustedFolders from '../../config/trustedFolders.js';
 
-vi.mock('process', () => ({
-  cwd: vi.fn(),
-  platform: 'linux',
-}));
+const mockedCwd = vi.hoisted(() => vi.fn());
+
+vi.mock('node:process', async () => {
+  const actual =
+    await vi.importActual<typeof import('node:process')>('node:process');
+  return {
+    ...actual,
+    cwd: mockedCwd,
+    platform: 'linux',
+  };
+});
 
 describe('useFolderTrust', () => {
   let mockSettings: LoadedSettings;
@@ -47,7 +52,7 @@ describe('useFolderTrust', () => {
       .spyOn(trustedFolders, 'loadTrustedFolders')
       .mockReturnValue(mockTrustedFolders);
     isWorkspaceTrustedSpy = vi.spyOn(trustedFolders, 'isWorkspaceTrusted');
-    (process.cwd as vi.Mock).mockReturnValue('/test/path');
+    mockedCwd.mockReturnValue('/test/path');
     onTrustChange = vi.fn();
   });