Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions packages/cli/src/acp/commands/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,26 +319,43 @@ export class UninstallExtensionCommand implements Command {
};
}

const name = args.join(' ').trim();
if (!name) {
const all = args.includes('--all');
const names = args.filter((a) => !a.startsWith('--')).map((a) => a.trim());

if (!all && names.length === 0) {
return {
name: this.name,
data: `Usage: /extensions uninstall <extension-name>`,
data: `Usage: /extensions uninstall <extension-names...>|--all`,
};
}

try {
await extensionLoader.uninstallExtension(name, false);
return {
name: this.name,
data: `Extension "${name}" uninstalled successfully.`,
};
} catch (error) {
let namesToUninstall: string[] = [];
if (all) {
namesToUninstall = extensionLoader.getExtensions().map((ext) => ext.name);
} else {
namesToUninstall = names;
}

if (namesToUninstall.length === 0) {
return {
name: this.name,
data: `Failed to uninstall extension "${name}": ${getErrorMessage(error)}`,
data: all ? 'No extensions installed.' : 'No extension name provided.',
};
}

const output: string[] = [];
for (const extensionName of namesToUninstall) {
try {
await extensionLoader.uninstallExtension(extensionName, false);
output.push(`Extension "${extensionName}" uninstalled successfully.`);
} catch (error) {
output.push(
`Failed to uninstall extension "${extensionName}": ${getErrorMessage(error)}`,
);
}
}

return { name: this.name, data: output.join('\n') };
}
}

Expand Down
76 changes: 68 additions & 8 deletions packages/cli/src/commands/extensions/uninstall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { getErrorMessage } from '../../utils/errors.js';
// Hoisted mocks - these survive vi.clearAllMocks()
const mockUninstallExtension = vi.hoisted(() => vi.fn());
const mockLoadExtensions = vi.hoisted(() => vi.fn());
const mockGetExtensions = vi.hoisted(() => vi.fn());

// Mock dependencies with hoisted functions
vi.mock('../../config/extension-manager.js', async (importOriginal) => {
Expand All @@ -38,6 +39,7 @@ vi.mock('../../config/extension-manager.js', async (importOriginal) => {
ExtensionManager: vi.fn().mockImplementation(() => ({
uninstallExtension: mockUninstallExtension,
loadExtensions: mockLoadExtensions,
getExtensions: mockGetExtensions,
setRequestConsent: vi.fn(),
setRequestSetting: vi.fn(),
})),
Expand Down Expand Up @@ -93,6 +95,7 @@ describe('extensions uninstall command', () => {
afterEach(() => {
mockLoadExtensions.mockClear();
mockUninstallExtension.mockClear();
mockGetExtensions.mockClear();
vi.clearAllMocks();
});

Expand Down Expand Up @@ -145,6 +148,41 @@ describe('extensions uninstall command', () => {
mockCwd.mockRestore();
});

it('should uninstall all extensions when --all flag is used', async () => {
mockLoadExtensions.mockResolvedValue(undefined);
mockUninstallExtension.mockResolvedValue(undefined);
mockGetExtensions.mockReturnValue([{ name: 'ext1' }, { name: 'ext2' }]);
const mockCwd = vi.spyOn(process, 'cwd').mockReturnValue('/test/dir');
await handleUninstall({ all: true });

expect(mockUninstallExtension).toHaveBeenCalledTimes(2);
expect(mockUninstallExtension).toHaveBeenCalledWith('ext1', false);
expect(mockUninstallExtension).toHaveBeenCalledWith('ext2', false);
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
'Extension "ext1" successfully uninstalled.',
);
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
'Extension "ext2" successfully uninstalled.',
);
mockCwd.mockRestore();
});

it('should log a message if no extensions are installed and --all flag is used', async () => {
mockLoadExtensions.mockResolvedValue(undefined);
mockGetExtensions.mockReturnValue([]);
const mockCwd = vi.spyOn(process, 'cwd').mockReturnValue('/test/dir');
await handleUninstall({ all: true });

expect(mockUninstallExtension).not.toHaveBeenCalled();
expect(emitConsoleLog).toHaveBeenCalledWith(
'log',
'No extensions currently installed.',
);
mockCwd.mockRestore();
});

it('should report errors for failed uninstalls but continue with others', async () => {
mockLoadExtensions.mockResolvedValue(undefined);
const mockCwd = vi.spyOn(process, 'cwd').mockReturnValue('/test/dir');
Expand Down Expand Up @@ -236,25 +274,27 @@ describe('extensions uninstall command', () => {
const command = uninstallCommand;

it('should have correct command and describe', () => {
expect(command.command).toBe('uninstall <names..>');
expect(command.command).toBe('uninstall [names..]');
expect(command.describe).toBe('Uninstalls one or more extensions.');
});

describe('builder', () => {
interface MockYargs {
positional: Mock;
option: Mock;
check: Mock;
}

let yargsMock: MockYargs;
beforeEach(() => {
yargsMock = {
positional: vi.fn().mockReturnThis(),
option: vi.fn().mockReturnThis(),
check: vi.fn().mockReturnThis(),
};
});

it('should configure positional argument', () => {
it('should configure arguments and options', () => {
(command.builder as (yargs: Argv) => Argv)(
yargsMock as unknown as Argv,
);
Expand All @@ -264,17 +304,30 @@ describe('extensions uninstall command', () => {
type: 'string',
array: true,
});
expect(yargsMock.option).toHaveBeenCalledWith('all', {
type: 'boolean',
describe: 'Uninstall all installed extensions.',
default: false,
});
expect(yargsMock.check).toHaveBeenCalled();
});

it('check function should throw for missing names', () => {
it('check function should throw for missing names and no --all flag', () => {
(command.builder as (yargs: Argv) => Argv)(
yargsMock as unknown as Argv,
);
const checkCallback = yargsMock.check.mock.calls[0][0];
expect(() => checkCallback({ names: [] })).toThrow(
'Please include at least one extension name to uninstall as a positional argument.',
expect(() => checkCallback({ names: [], all: false })).toThrow(
'Please include at least one extension name to uninstall as a positional argument, or use the --all flag.',
);
});

it('check function should pass if --all flag is used even without names', () => {
(command.builder as (yargs: Argv) => Argv)(
yargsMock as unknown as Argv,
);
const checkCallback = yargsMock.check.mock.calls[0][0];
expect(() => checkCallback({ names: [], all: true })).not.toThrow();
});
});

Expand All @@ -283,10 +336,17 @@ describe('extensions uninstall command', () => {
mockUninstallExtension.mockResolvedValue(undefined);
const mockCwd = vi.spyOn(process, 'cwd').mockReturnValue('/test/dir');
interface TestArgv {
names: string[];
[key: string]: unknown;
names?: string[];
all?: boolean;
_: string[];
$0: string;
}
const argv: TestArgv = { names: ['my-extension'], _: [], $0: '' };
const argv: TestArgv = {
names: ['my-extension'],
all: false,
_: [],
$0: '',
};
await (command.handler as unknown as (args: TestArgv) => Promise<void>)(
argv,
);
Expand Down
36 changes: 30 additions & 6 deletions packages/cli/src/commands/extensions/uninstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';

interface UninstallArgs {
names: string[]; // can be extension names or source URLs.
names?: string[]; // can be extension names or source URLs.
all?: boolean;
}

export async function handleUninstall(args: UninstallArgs) {
Expand All @@ -28,8 +29,24 @@ export async function handleUninstall(args: UninstallArgs) {
});
await extensionManager.loadExtensions();

let namesToUninstall: string[] = [];
if (args.all) {
namesToUninstall = extensionManager
.getExtensions()
.map((ext) => ext.name);
} else if (args.names) {
namesToUninstall = [...new Set(args.names)];
}

if (namesToUninstall.length === 0) {
if (args.all) {
debugLogger.log('No extensions currently installed.');
}
return;
}

const errors: Array<{ name: string; error: string }> = [];
for (const name of [...new Set(args.names)]) {
for (const name of namesToUninstall) {
try {
await extensionManager.uninstallExtension(name, false);
debugLogger.log(`Extension "${name}" successfully uninstalled.`);
Expand All @@ -51,7 +68,7 @@ export async function handleUninstall(args: UninstallArgs) {
}

export const uninstallCommand: CommandModule = {
command: 'uninstall <names..>',
command: 'uninstall [names..]',
describe: 'Uninstalls one or more extensions.',
builder: (yargs) =>
yargs
Expand All @@ -61,18 +78,25 @@ export const uninstallCommand: CommandModule = {
type: 'string',
array: true,
})
.option('all', {
type: 'boolean',
describe: 'Uninstall all installed extensions.',
default: false,
})
.check((argv) => {
if (!argv.names || argv.names.length === 0) {
if (!argv.all && (!argv.names || argv.names.length === 0)) {
throw new Error(
'Please include at least one extension name to uninstall as a positional argument.',
'Please include at least one extension name to uninstall as a positional argument, or use the --all flag.',
);
}
return true;
}),
handler: async (argv) => {
await handleUninstall({
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
names: argv['names'] as string[],
names: argv['names'] as string[] | undefined,
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
all: argv['all'] as boolean,
});
await exitCli();
},
Expand Down
16 changes: 16 additions & 0 deletions packages/cli/src/ui/AppContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ vi.mock('./hooks/useIdeTrustListener.js');
vi.mock('./hooks/useMessageQueue.js');
vi.mock('./hooks/useApprovalModeIndicator.js');
vi.mock('./hooks/useGitBranchName.js');
vi.mock('./hooks/useExtensionUpdates.js');
vi.mock('./contexts/VimModeContext.js');
vi.mock('./contexts/SessionContext.js');
vi.mock('./components/shared/text-buffer.js');
Expand Down Expand Up @@ -218,6 +219,10 @@ import { useIdeTrustListener } from './hooks/useIdeTrustListener.js';
import { useMessageQueue } from './hooks/useMessageQueue.js';
import { useApprovalModeIndicator } from './hooks/useApprovalModeIndicator.js';
import { useGitBranchName } from './hooks/useGitBranchName.js';
import {
useConfirmUpdateRequests,
useExtensionUpdates,
} from './hooks/useExtensionUpdates.js';
import { useVimMode } from './contexts/VimModeContext.js';
import { useSessionStats } from './contexts/SessionContext.js';
import { useTextBuffer } from './components/shared/text-buffer.js';
Expand Down Expand Up @@ -299,6 +304,8 @@ describe('AppContainer State Management', () => {
const mockedUseMessageQueue = useMessageQueue as Mock;
const mockedUseApprovalModeIndicator = useApprovalModeIndicator as Mock;
const mockedUseGitBranchName = useGitBranchName as Mock;
const mockedUseConfirmUpdateRequests = useConfirmUpdateRequests as Mock;
const mockedUseExtensionUpdates = useExtensionUpdates as Mock;
const mockedUseVimMode = useVimMode as Mock;
const mockedUseSessionStats = useSessionStats as Mock;
const mockedUseTextBuffer = useTextBuffer as Mock;
Expand Down Expand Up @@ -451,6 +458,15 @@ describe('AppContainer State Management', () => {
isFocused: true,
hasReceivedFocusEvent: true,
});
mockedUseConfirmUpdateRequests.mockReturnValue({
addConfirmUpdateExtensionRequest: vi.fn(),
confirmUpdateExtensionRequests: [],
});
mockedUseExtensionUpdates.mockReturnValue({
extensionsUpdateState: new Map(),
extensionsUpdateStateInternal: new Map(),
dispatchExtensionStateUpdate: vi.fn(),
});

// Mock Config
mockConfig = makeFakeConfig();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/commands/extensionsCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ describe('extensionsCommand', () => {
await uninstallAction!(mockContext, '');
expect(mockContext.ui.addItem).toHaveBeenCalledWith({
type: MessageType.ERROR,
text: 'Usage: /extensions uninstall <extension-name>',
text: 'Usage: /extensions uninstall <extension-names...>|--all',
});
expect(mockUninstallExtension).not.toHaveBeenCalled();
});
Expand Down
Loading
Loading