Skip to content

Commit feeee4f

Browse files
chrstnbjk-kashe
authored andcommitted
Remove separate --path argument for extensions install command (google-gemini#10628)
1 parent 11edeb0 commit feeee4f

File tree

4 files changed

+48
-77
lines changed

4 files changed

+48
-77
lines changed

docs/extensions/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Note that all of these commands will only be reflected in active CLI sessions on
1818

1919
### Installing an extension
2020

21-
You can install an extension using `gemini extensions install` with either a GitHub URL source or `--path=some/local/path`.
21+
You can install an extension using `gemini extensions install` with either a GitHub URL or a local path`.
2222

2323
Note that we create a copy of the installed extension, so you will need to run `gemini extensions update` to pull in changes from both locally-defined extensions and those on GitHub.
2424

integration-tests/extensions-install.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ test('installs a local extension, verifies a command, and updates it', async ()
3131
}
3232

3333
const result = await rig.runCommand(
34-
['extensions', 'install', `--path=${rig.testDir!}`],
34+
['extensions', 'install', `${rig.testDir!}`],
3535
{ stdin: 'y\n' },
3636
);
3737
expect(result).toContain('test-extension');

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

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import yargs from 'yargs';
1010

1111
const mockInstallExtension = vi.hoisted(() => vi.fn());
1212
const mockRequestConsentNonInteractive = vi.hoisted(() => vi.fn());
13+
const mockStat = vi.hoisted(() => vi.fn());
1314

1415
vi.mock('../../config/extension.js', () => ({
1516
installExtension: mockInstallExtension,
@@ -20,35 +21,20 @@ vi.mock('../../utils/errors.js', () => ({
2021
getErrorMessage: vi.fn((error: Error) => error.message),
2122
}));
2223

24+
vi.mock('node:fs/promises', () => ({
25+
stat: mockStat,
26+
default: {
27+
stat: mockStat,
28+
},
29+
}));
30+
2331
describe('extensions install command', () => {
2432
it('should fail if no source is provided', () => {
2533
const validationParser = yargs([]).command(installCommand).fail(false);
2634
expect(() => validationParser.parse('install')).toThrow(
27-
'Either source or --path must be provided.',
35+
'Not enough non-option arguments: got 0, need at least 1',
2836
);
2937
});
30-
31-
it('should fail if both git source and local path are provided', () => {
32-
const validationParser = yargs([])
33-
.command(installCommand)
34-
.fail(false)
35-
.locale('en');
36-
expect(() =>
37-
validationParser.parse('install some-url --path /some/path'),
38-
).toThrow('Arguments source and path are mutually exclusive');
39-
});
40-
41-
it('should fail if both auto update and local path are provided', () => {
42-
const validationParser = yargs([])
43-
.command(installCommand)
44-
.fail(false)
45-
.locale('en');
46-
expect(() =>
47-
validationParser.parse(
48-
'install some-url --path /some/path --auto-update',
49-
),
50-
).toThrow('Arguments path and auto-update are mutually exclusive');
51-
});
5238
});
5339

5440
describe('handleInstall', () => {
@@ -67,6 +53,7 @@ describe('handleInstall', () => {
6753
afterEach(() => {
6854
mockInstallExtension.mockClear();
6955
mockRequestConsentNonInteractive.mockClear();
56+
mockStat.mockClear();
7057
vi.resetAllMocks();
7158
});
7259

@@ -107,13 +94,12 @@ describe('handleInstall', () => {
10794
});
10895

10996
it('throws an error from an unknown source', async () => {
97+
mockStat.mockRejectedValue(new Error('ENOENT: no such file or directory'));
11098
await handleInstall({
11199
source: 'test://google.com',
112100
});
113101

114-
expect(consoleErrorSpy).toHaveBeenCalledWith(
115-
'The source "test://google.com" is not a valid URL format.',
116-
);
102+
expect(consoleErrorSpy).toHaveBeenCalledWith('Install source not found.');
117103
expect(processSpy).toHaveBeenCalledWith(1);
118104
});
119105

@@ -131,25 +117,16 @@ describe('handleInstall', () => {
131117

132118
it('should install an extension from a local path', async () => {
133119
mockInstallExtension.mockResolvedValue('local-extension');
134-
120+
mockStat.mockResolvedValue({});
135121
await handleInstall({
136-
path: '/some/path',
122+
source: '/some/path',
137123
});
138124

139125
expect(consoleLogSpy).toHaveBeenCalledWith(
140126
'Extension "local-extension" installed successfully and enabled.',
141127
);
142128
});
143129

144-
it('should throw an error if no source or path is provided', async () => {
145-
await handleInstall({});
146-
147-
expect(consoleErrorSpy).toHaveBeenCalledWith(
148-
'Either --source or --path must be provided.',
149-
);
150-
expect(processSpy).toHaveBeenCalledWith(1);
151-
});
152-
153130
it('should throw an error if install extension fails', async () => {
154131
mockInstallExtension.mockRejectedValue(
155132
new Error('Install extension failed'),

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

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,45 +10,46 @@ import {
1010
requestConsentNonInteractive,
1111
} from '../../config/extension.js';
1212
import type { ExtensionInstallMetadata } from '@google/gemini-cli-core';
13-
1413
import { getErrorMessage } from '../../utils/errors.js';
14+
import { stat } from 'node:fs/promises';
1515

1616
interface InstallArgs {
17-
source?: string;
18-
path?: string;
17+
source: string;
1918
ref?: string;
2019
autoUpdate?: boolean;
2120
}
2221

2322
export async function handleInstall(args: InstallArgs) {
2423
try {
2524
let installMetadata: ExtensionInstallMetadata;
26-
if (args.source) {
27-
const { source } = args;
28-
if (
29-
source.startsWith('http://') ||
30-
source.startsWith('https://') ||
31-
source.startsWith('git@') ||
32-
source.startsWith('sso://')
33-
) {
34-
installMetadata = {
35-
source,
36-
type: 'git',
37-
ref: args.ref,
38-
autoUpdate: args.autoUpdate,
39-
};
40-
} else {
41-
throw new Error(`The source "${source}" is not a valid URL format.`);
42-
}
43-
} else if (args.path) {
25+
const { source } = args;
26+
if (
27+
source.startsWith('http://') ||
28+
source.startsWith('https://') ||
29+
source.startsWith('git@') ||
30+
source.startsWith('sso://')
31+
) {
4432
installMetadata = {
45-
source: args.path,
46-
type: 'local',
33+
source,
34+
type: 'git',
35+
ref: args.ref,
4736
autoUpdate: args.autoUpdate,
4837
};
4938
} else {
50-
// This should not be reached due to the yargs check.
51-
throw new Error('Either --source or --path must be provided.');
39+
if (args.ref || args.autoUpdate) {
40+
throw new Error(
41+
'--ref and --auto-update are not applicable for local extensions.',
42+
);
43+
}
44+
try {
45+
await stat(source);
46+
installMetadata = {
47+
source,
48+
type: 'local',
49+
};
50+
} catch {
51+
throw new Error('Install source not found.');
52+
}
5253
}
5354

5455
const name = await installExtension(
@@ -63,17 +64,14 @@ export async function handleInstall(args: InstallArgs) {
6364
}
6465

6566
export const installCommand: CommandModule = {
66-
command: 'install [<source>] [--path] [--ref] [--auto-update]',
67+
command: 'install <source>',
6768
describe: 'Installs an extension from a git repository URL or a local path.',
6869
builder: (yargs) =>
6970
yargs
7071
.positional('source', {
71-
describe: 'The github URL of the extension to install.',
72-
type: 'string',
73-
})
74-
.option('path', {
75-
describe: 'Path to a local extension directory.',
72+
describe: 'The github URL or local path of the extension to install.',
7673
type: 'string',
74+
demandOption: true,
7775
})
7876
.option('ref', {
7977
describe: 'The git ref to install from.',
@@ -83,19 +81,15 @@ export const installCommand: CommandModule = {
8381
describe: 'Enable auto-update for this extension.',
8482
type: 'boolean',
8583
})
86-
.conflicts('source', 'path')
87-
.conflicts('path', 'ref')
88-
.conflicts('path', 'auto-update')
8984
.check((argv) => {
90-
if (!argv.source && !argv.path) {
91-
throw new Error('Either source or --path must be provided.');
85+
if (!argv.source) {
86+
throw new Error('The source argument must be provided.');
9287
}
9388
return true;
9489
}),
9590
handler: async (argv) => {
9691
await handleInstall({
97-
source: argv['source'] as string | undefined,
98-
path: argv['path'] as string | undefined,
92+
source: argv['source'] as string,
9993
ref: argv['ref'] as string | undefined,
10094
autoUpdate: argv['auto-update'] as boolean | undefined,
10195
});

0 commit comments

Comments
 (0)