diff --git a/packages/cli/src/services/BuiltinCommandLoader.test.ts b/packages/cli/src/services/BuiltinCommandLoader.test.ts new file mode 100644 index 00000000..642309dc --- /dev/null +++ b/packages/cli/src/services/BuiltinCommandLoader.test.ts @@ -0,0 +1,118 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +vi.mock('../ui/commands/aboutCommand.js', async () => { + const { CommandKind } = await import('../ui/commands/types.js'); + return { + aboutCommand: { + name: 'about', + description: 'About the CLI', + kind: CommandKind.BUILT_IN, + }, + }; +}); + +vi.mock('../ui/commands/ideCommand.js', () => ({ ideCommand: vi.fn() })); +vi.mock('../ui/commands/restoreCommand.js', () => ({ + restoreCommand: vi.fn(), +})); + +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { BuiltinCommandLoader } from './BuiltinCommandLoader.js'; +import { Config } from '@google/gemini-cli-core'; +import { CommandKind } from '../ui/commands/types.js'; + +import { ideCommand } from '../ui/commands/ideCommand.js'; +import { restoreCommand } from '../ui/commands/restoreCommand.js'; + +vi.mock('../ui/commands/authCommand.js', () => ({ authCommand: {} })); +vi.mock('../ui/commands/bugCommand.js', () => ({ bugCommand: {} })); +vi.mock('../ui/commands/chatCommand.js', () => ({ chatCommand: {} })); +vi.mock('../ui/commands/clearCommand.js', () => ({ clearCommand: {} })); +vi.mock('../ui/commands/compressCommand.js', () => ({ compressCommand: {} })); +vi.mock('../ui/commands/corgiCommand.js', () => ({ corgiCommand: {} })); +vi.mock('../ui/commands/docsCommand.js', () => ({ docsCommand: {} })); +vi.mock('../ui/commands/editorCommand.js', () => ({ editorCommand: {} })); +vi.mock('../ui/commands/extensionsCommand.js', () => ({ + extensionsCommand: {}, +})); +vi.mock('../ui/commands/helpCommand.js', () => ({ helpCommand: {} })); +vi.mock('../ui/commands/mcpCommand.js', () => ({ mcpCommand: {} })); +vi.mock('../ui/commands/memoryCommand.js', () => ({ memoryCommand: {} })); +vi.mock('../ui/commands/privacyCommand.js', () => ({ privacyCommand: {} })); +vi.mock('../ui/commands/quitCommand.js', () => ({ quitCommand: {} })); +vi.mock('../ui/commands/statsCommand.js', () => ({ statsCommand: {} })); +vi.mock('../ui/commands/themeCommand.js', () => ({ themeCommand: {} })); +vi.mock('../ui/commands/toolsCommand.js', () => ({ toolsCommand: {} })); + +describe('BuiltinCommandLoader', () => { + let mockConfig: Config; + + const ideCommandMock = ideCommand as Mock; + const restoreCommandMock = restoreCommand as Mock; + + beforeEach(() => { + vi.clearAllMocks(); + mockConfig = { some: 'config' } as unknown as Config; + + ideCommandMock.mockReturnValue({ + name: 'ide', + description: 'IDE command', + kind: CommandKind.BUILT_IN, + }); + restoreCommandMock.mockReturnValue({ + name: 'restore', + description: 'Restore command', + kind: CommandKind.BUILT_IN, + }); + }); + + it('should correctly pass the config object to command factory functions', async () => { + const loader = new BuiltinCommandLoader(mockConfig); + await loader.loadCommands(); + + expect(ideCommandMock).toHaveBeenCalledTimes(1); + expect(ideCommandMock).toHaveBeenCalledWith(mockConfig); + expect(restoreCommandMock).toHaveBeenCalledTimes(1); + expect(restoreCommandMock).toHaveBeenCalledWith(mockConfig); + }); + + it('should filter out null command definitions returned by factories', async () => { + // Override the mock's behavior for this specific test. + ideCommandMock.mockReturnValue(null); + const loader = new BuiltinCommandLoader(mockConfig); + const commands = await loader.loadCommands(); + + // The 'ide' command should be filtered out. + const ideCmd = commands.find((c) => c.name === 'ide'); + expect(ideCmd).toBeUndefined(); + + // Other commands should still be present. + const aboutCmd = commands.find((c) => c.name === 'about'); + expect(aboutCmd).toBeDefined(); + }); + + it('should handle a null config gracefully when calling factories', async () => { + const loader = new BuiltinCommandLoader(null); + await loader.loadCommands(); + expect(ideCommandMock).toHaveBeenCalledTimes(1); + expect(ideCommandMock).toHaveBeenCalledWith(null); + expect(restoreCommandMock).toHaveBeenCalledTimes(1); + expect(restoreCommandMock).toHaveBeenCalledWith(null); + }); + + it('should return a list of all loaded commands', async () => { + const loader = new BuiltinCommandLoader(mockConfig); + const commands = await loader.loadCommands(); + + const aboutCmd = commands.find((c) => c.name === 'about'); + expect(aboutCmd).toBeDefined(); + expect(aboutCmd?.kind).toBe(CommandKind.BUILT_IN); + + const ideCmd = commands.find((c) => c.name === 'ide'); + expect(ideCmd).toBeDefined(); + }); +}); diff --git a/packages/cli/src/services/BuiltinCommandLoader.ts b/packages/cli/src/services/BuiltinCommandLoader.ts new file mode 100644 index 00000000..259c6013 --- /dev/null +++ b/packages/cli/src/services/BuiltinCommandLoader.ts @@ -0,0 +1,73 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ICommandLoader } from './types.js'; +import { SlashCommand } from '../ui/commands/types.js'; +import { Config } from '@google/gemini-cli-core'; +import { aboutCommand } from '../ui/commands/aboutCommand.js'; +import { authCommand } from '../ui/commands/authCommand.js'; +import { bugCommand } from '../ui/commands/bugCommand.js'; +import { chatCommand } from '../ui/commands/chatCommand.js'; +import { clearCommand } from '../ui/commands/clearCommand.js'; +import { compressCommand } from '../ui/commands/compressCommand.js'; +import { copyCommand } from '../ui/commands/copyCommand.js'; +import { corgiCommand } from '../ui/commands/corgiCommand.js'; +import { docsCommand } from '../ui/commands/docsCommand.js'; +import { editorCommand } from '../ui/commands/editorCommand.js'; +import { extensionsCommand } from '../ui/commands/extensionsCommand.js'; +import { helpCommand } from '../ui/commands/helpCommand.js'; +import { ideCommand } from '../ui/commands/ideCommand.js'; +import { mcpCommand } from '../ui/commands/mcpCommand.js'; +import { memoryCommand } from '../ui/commands/memoryCommand.js'; +import { privacyCommand } from '../ui/commands/privacyCommand.js'; +import { quitCommand } from '../ui/commands/quitCommand.js'; +import { restoreCommand } from '../ui/commands/restoreCommand.js'; +import { statsCommand } from '../ui/commands/statsCommand.js'; +import { themeCommand } from '../ui/commands/themeCommand.js'; +import { toolsCommand } from '../ui/commands/toolsCommand.js'; + +/** + * Loads the core, hard-coded slash commands that are an integral part + * of the Gemini CLI application. + */ +export class BuiltinCommandLoader implements ICommandLoader { + constructor(private config: Config | null) {} + + /** + * Gathers all raw built-in command definitions, injects dependencies where + * needed (e.g., config) and filters out any that are not available. + * + * @param _signal An AbortSignal (unused for this synchronous loader). + * @returns A promise that resolves to an array of `SlashCommand` objects. + */ + async loadCommands(_signal: AbortSignal): Promise { + const allDefinitions: Array = [ + aboutCommand, + authCommand, + bugCommand, + chatCommand, + clearCommand, + compressCommand, + copyCommand, + corgiCommand, + docsCommand, + editorCommand, + extensionsCommand, + helpCommand, + ideCommand(this.config), + mcpCommand, + memoryCommand, + privacyCommand, + quitCommand, + restoreCommand(this.config), + statsCommand, + themeCommand, + toolsCommand, + ]; + + return allDefinitions.filter((cmd): cmd is SlashCommand => cmd !== null); + } +} diff --git a/packages/cli/src/services/CommandService.test.ts b/packages/cli/src/services/CommandService.test.ts index de4ff2ea..28731f81 100644 --- a/packages/cli/src/services/CommandService.test.ts +++ b/packages/cli/src/services/CommandService.test.ts @@ -4,254 +4,177 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, describe, it, expect, beforeEach, type Mocked } from 'vitest'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; import { CommandService } from './CommandService.js'; -import { type Config } from '@google/gemini-cli-core'; -import { type SlashCommand } from '../ui/commands/types.js'; -import { memoryCommand } from '../ui/commands/memoryCommand.js'; -import { helpCommand } from '../ui/commands/helpCommand.js'; -import { clearCommand } from '../ui/commands/clearCommand.js'; -import { copyCommand } from '../ui/commands/copyCommand.js'; -import { corgiCommand } from '../ui/commands/corgiCommand.js'; -import { docsCommand } from '../ui/commands/docsCommand.js'; -import { chatCommand } from '../ui/commands/chatCommand.js'; -import { authCommand } from '../ui/commands/authCommand.js'; -import { themeCommand } from '../ui/commands/themeCommand.js'; -import { statsCommand } from '../ui/commands/statsCommand.js'; -import { privacyCommand } from '../ui/commands/privacyCommand.js'; -import { aboutCommand } from '../ui/commands/aboutCommand.js'; -import { ideCommand } from '../ui/commands/ideCommand.js'; -import { extensionsCommand } from '../ui/commands/extensionsCommand.js'; -import { toolsCommand } from '../ui/commands/toolsCommand.js'; -import { compressCommand } from '../ui/commands/compressCommand.js'; -import { mcpCommand } from '../ui/commands/mcpCommand.js'; -import { editorCommand } from '../ui/commands/editorCommand.js'; -import { bugCommand } from '../ui/commands/bugCommand.js'; -import { quitCommand } from '../ui/commands/quitCommand.js'; -import { restoreCommand } from '../ui/commands/restoreCommand.js'; +import { type ICommandLoader } from './types.js'; +import { CommandKind, type SlashCommand } from '../ui/commands/types.js'; -// Mock the command modules to isolate the service from the command implementations. -vi.mock('../ui/commands/memoryCommand.js', () => ({ - memoryCommand: { name: 'memory', description: 'Mock Memory' }, -})); -vi.mock('../ui/commands/helpCommand.js', () => ({ - helpCommand: { name: 'help', description: 'Mock Help' }, -})); -vi.mock('../ui/commands/clearCommand.js', () => ({ - clearCommand: { name: 'clear', description: 'Mock Clear' }, -})); -vi.mock('../ui/commands/corgiCommand.js', () => ({ - corgiCommand: { name: 'corgi', description: 'Mock Corgi' }, -})); -vi.mock('../ui/commands/docsCommand.js', () => ({ - docsCommand: { name: 'docs', description: 'Mock Docs' }, -})); -vi.mock('../ui/commands/authCommand.js', () => ({ - authCommand: { name: 'auth', description: 'Mock Auth' }, -})); -vi.mock('../ui/commands/themeCommand.js', () => ({ - themeCommand: { name: 'theme', description: 'Mock Theme' }, -})); -vi.mock('../ui/commands/copyCommand.js', () => ({ - copyCommand: { name: 'copy', description: 'Mock Copy' }, -})); -vi.mock('../ui/commands/privacyCommand.js', () => ({ - privacyCommand: { name: 'privacy', description: 'Mock Privacy' }, -})); -vi.mock('../ui/commands/statsCommand.js', () => ({ - statsCommand: { name: 'stats', description: 'Mock Stats' }, -})); -vi.mock('../ui/commands/aboutCommand.js', () => ({ - aboutCommand: { name: 'about', description: 'Mock About' }, -})); -vi.mock('../ui/commands/ideCommand.js', () => ({ - ideCommand: vi.fn(), -})); -vi.mock('../ui/commands/extensionsCommand.js', () => ({ - extensionsCommand: { name: 'extensions', description: 'Mock Extensions' }, -})); -vi.mock('../ui/commands/toolsCommand.js', () => ({ - toolsCommand: { name: 'tools', description: 'Mock Tools' }, -})); -vi.mock('../ui/commands/compressCommand.js', () => ({ - compressCommand: { name: 'compress', description: 'Mock Compress' }, -})); -vi.mock('../ui/commands/mcpCommand.js', () => ({ - mcpCommand: { name: 'mcp', description: 'Mock MCP' }, -})); -vi.mock('../ui/commands/editorCommand.js', () => ({ - editorCommand: { name: 'editor', description: 'Mock Editor' }, -})); -vi.mock('../ui/commands/bugCommand.js', () => ({ - bugCommand: { name: 'bug', description: 'Mock Bug' }, -})); -vi.mock('../ui/commands/quitCommand.js', () => ({ - quitCommand: { name: 'quit', description: 'Mock Quit' }, -})); -vi.mock('../ui/commands/restoreCommand.js', () => ({ - restoreCommand: vi.fn(), -})); +const createMockCommand = (name: string, kind: CommandKind): SlashCommand => ({ + name, + description: `Description for ${name}`, + kind, + action: vi.fn(), +}); + +const mockCommandA = createMockCommand('command-a', CommandKind.BUILT_IN); +const mockCommandB = createMockCommand('command-b', CommandKind.BUILT_IN); +const mockCommandC = createMockCommand('command-c', CommandKind.FILE); +const mockCommandB_Override = createMockCommand('command-b', CommandKind.FILE); + +class MockCommandLoader implements ICommandLoader { + private commandsToLoad: SlashCommand[]; + + constructor(commandsToLoad: SlashCommand[]) { + this.commandsToLoad = commandsToLoad; + } + + loadCommands = vi.fn( + async (): Promise => Promise.resolve(this.commandsToLoad), + ); +} describe('CommandService', () => { - const subCommandLen = 19; - let mockConfig: Mocked; - beforeEach(() => { - mockConfig = { - getIdeMode: vi.fn(), - getCheckpointingEnabled: vi.fn(), - } as unknown as Mocked; - vi.mocked(ideCommand).mockReturnValue(null); - vi.mocked(restoreCommand).mockReturnValue(null); + vi.spyOn(console, 'debug').mockImplementation(() => {}); }); - describe('when using default production loader', () => { - let commandService: CommandService; - - beforeEach(() => { - commandService = new CommandService(mockConfig); - }); - - it('should initialize with an empty command tree', () => { - const tree = commandService.getCommands(); - expect(tree).toBeInstanceOf(Array); - expect(tree.length).toBe(0); - }); - - describe('loadCommands', () => { - it('should load the built-in commands into the command tree', async () => { - // Pre-condition check - expect(commandService.getCommands().length).toBe(0); - - // Action - await commandService.loadCommands(); - const tree = commandService.getCommands(); - - // Post-condition assertions - expect(tree.length).toBe(subCommandLen); - - const commandNames = tree.map((cmd) => cmd.name); - expect(commandNames).toContain('auth'); - expect(commandNames).toContain('bug'); - expect(commandNames).toContain('memory'); - expect(commandNames).toContain('help'); - expect(commandNames).toContain('clear'); - expect(commandNames).toContain('copy'); - expect(commandNames).toContain('compress'); - expect(commandNames).toContain('corgi'); - expect(commandNames).toContain('docs'); - expect(commandNames).toContain('chat'); - expect(commandNames).toContain('theme'); - expect(commandNames).toContain('stats'); - expect(commandNames).toContain('privacy'); - expect(commandNames).toContain('about'); - expect(commandNames).toContain('extensions'); - expect(commandNames).toContain('tools'); - expect(commandNames).toContain('mcp'); - expect(commandNames).not.toContain('ide'); - }); - - it('should include ide command when ideMode is on', async () => { - mockConfig.getIdeMode.mockReturnValue(true); - vi.mocked(ideCommand).mockReturnValue({ - name: 'ide', - description: 'Mock IDE', - }); - await commandService.loadCommands(); - const tree = commandService.getCommands(); - - expect(tree.length).toBe(subCommandLen + 1); - const commandNames = tree.map((cmd) => cmd.name); - expect(commandNames).toContain('ide'); - expect(commandNames).toContain('editor'); - expect(commandNames).toContain('quit'); - }); - - it('should include restore command when checkpointing is on', async () => { - mockConfig.getCheckpointingEnabled.mockReturnValue(true); - vi.mocked(restoreCommand).mockReturnValue({ - name: 'restore', - description: 'Mock Restore', - }); - await commandService.loadCommands(); - const tree = commandService.getCommands(); - - expect(tree.length).toBe(subCommandLen + 1); - const commandNames = tree.map((cmd) => cmd.name); - expect(commandNames).toContain('restore'); - }); - - it('should overwrite any existing commands when called again', async () => { - // Load once - await commandService.loadCommands(); - expect(commandService.getCommands().length).toBe(subCommandLen); - - // Load again - await commandService.loadCommands(); - const tree = commandService.getCommands(); - - // Should not append, but overwrite - expect(tree.length).toBe(subCommandLen); - }); - }); - - describe('getCommandTree', () => { - it('should return the current command tree', async () => { - const initialTree = commandService.getCommands(); - expect(initialTree).toEqual([]); - - await commandService.loadCommands(); - - const loadedTree = commandService.getCommands(); - expect(loadedTree.length).toBe(subCommandLen); - expect(loadedTree).toEqual([ - aboutCommand, - authCommand, - bugCommand, - chatCommand, - clearCommand, - copyCommand, - compressCommand, - corgiCommand, - docsCommand, - editorCommand, - extensionsCommand, - helpCommand, - mcpCommand, - memoryCommand, - privacyCommand, - quitCommand, - statsCommand, - themeCommand, - toolsCommand, - ]); - }); - }); + afterEach(() => { + vi.restoreAllMocks(); }); - describe('when initialized with an injected loader function', () => { - it('should use the provided loader instead of the built-in one', async () => { - // Arrange: Create a set of mock commands. - const mockCommands: SlashCommand[] = [ - { name: 'injected-test-1', description: 'injected 1' }, - { name: 'injected-test-2', description: 'injected 2' }, - ]; + it('should load commands from a single loader', async () => { + const mockLoader = new MockCommandLoader([mockCommandA, mockCommandB]); + const service = await CommandService.create( + [mockLoader], + new AbortController().signal, + ); - // Arrange: Create a mock loader FUNCTION that resolves with our mock commands. - const mockLoader = vi.fn().mockResolvedValue(mockCommands); + const commands = service.getCommands(); - // Act: Instantiate the service WITH the injected loader function. - const commandService = new CommandService(mockConfig, mockLoader); - await commandService.loadCommands(); - const tree = commandService.getCommands(); + expect(mockLoader.loadCommands).toHaveBeenCalledTimes(1); + expect(commands).toHaveLength(2); + expect(commands).toEqual( + expect.arrayContaining([mockCommandA, mockCommandB]), + ); + }); - // Assert: The tree should contain ONLY our injected commands. - expect(mockLoader).toHaveBeenCalled(); // Verify our mock loader was actually called. - expect(tree.length).toBe(2); - expect(tree).toEqual(mockCommands); + it('should aggregate commands from multiple loaders', async () => { + const loader1 = new MockCommandLoader([mockCommandA]); + const loader2 = new MockCommandLoader([mockCommandC]); + const service = await CommandService.create( + [loader1, loader2], + new AbortController().signal, + ); - const commandNames = tree.map((cmd) => cmd.name); - expect(commandNames).not.toContain('memory'); // Verify it didn't load production commands. - }); + const commands = service.getCommands(); + + expect(loader1.loadCommands).toHaveBeenCalledTimes(1); + expect(loader2.loadCommands).toHaveBeenCalledTimes(1); + expect(commands).toHaveLength(2); + expect(commands).toEqual( + expect.arrayContaining([mockCommandA, mockCommandC]), + ); + }); + + it('should override commands from earlier loaders with those from later loaders', async () => { + const loader1 = new MockCommandLoader([mockCommandA, mockCommandB]); + const loader2 = new MockCommandLoader([ + mockCommandB_Override, + mockCommandC, + ]); + const service = await CommandService.create( + [loader1, loader2], + new AbortController().signal, + ); + + const commands = service.getCommands(); + + expect(commands).toHaveLength(3); // Should be A, C, and the overridden B. + + // The final list should contain the override from the *last* loader. + const commandB = commands.find((cmd) => cmd.name === 'command-b'); + expect(commandB).toBeDefined(); + expect(commandB?.kind).toBe(CommandKind.FILE); // Verify it's the overridden version. + expect(commandB).toEqual(mockCommandB_Override); + + // Ensure the other commands are still present. + expect(commands).toEqual( + expect.arrayContaining([ + mockCommandA, + mockCommandC, + mockCommandB_Override, + ]), + ); + }); + + it('should handle loaders that return an empty array of commands gracefully', async () => { + const loader1 = new MockCommandLoader([mockCommandA]); + const emptyLoader = new MockCommandLoader([]); + const loader3 = new MockCommandLoader([mockCommandB]); + const service = await CommandService.create( + [loader1, emptyLoader, loader3], + new AbortController().signal, + ); + + const commands = service.getCommands(); + + expect(emptyLoader.loadCommands).toHaveBeenCalledTimes(1); + expect(commands).toHaveLength(2); + expect(commands).toEqual( + expect.arrayContaining([mockCommandA, mockCommandB]), + ); + }); + + it('should load commands from successful loaders even if one fails', async () => { + const successfulLoader = new MockCommandLoader([mockCommandA]); + const failingLoader = new MockCommandLoader([]); + const error = new Error('Loader failed'); + vi.spyOn(failingLoader, 'loadCommands').mockRejectedValue(error); + + const service = await CommandService.create( + [successfulLoader, failingLoader], + new AbortController().signal, + ); + + const commands = service.getCommands(); + expect(commands).toHaveLength(1); + expect(commands).toEqual([mockCommandA]); + expect(console.debug).toHaveBeenCalledWith( + 'A command loader failed:', + error, + ); + }); + + it('getCommands should return a readonly array that cannot be mutated', async () => { + const service = await CommandService.create( + [new MockCommandLoader([mockCommandA])], + new AbortController().signal, + ); + + const commands = service.getCommands(); + + // Expect it to throw a TypeError at runtime because the array is frozen. + expect(() => { + // @ts-expect-error - Testing immutability is intentional here. + commands.push(mockCommandB); + }).toThrow(); + + // Verify the original array was not mutated. + expect(service.getCommands()).toHaveLength(1); + }); + + it('should pass the abort signal to all loaders', async () => { + const controller = new AbortController(); + const signal = controller.signal; + + const loader1 = new MockCommandLoader([mockCommandA]); + const loader2 = new MockCommandLoader([mockCommandB]); + + await CommandService.create([loader1, loader2], signal); + + expect(loader1.loadCommands).toHaveBeenCalledTimes(1); + expect(loader1.loadCommands).toHaveBeenCalledWith(signal); + expect(loader2.loadCommands).toHaveBeenCalledTimes(1); + expect(loader2.loadCommands).toHaveBeenCalledWith(signal); }); }); diff --git a/packages/cli/src/services/CommandService.ts b/packages/cli/src/services/CommandService.ts index 99eccbf2..ef4f4d14 100644 --- a/packages/cli/src/services/CommandService.ts +++ b/packages/cli/src/services/CommandService.ts @@ -4,81 +4,80 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Config } from '@google/gemini-cli-core'; import { SlashCommand } from '../ui/commands/types.js'; -import { memoryCommand } from '../ui/commands/memoryCommand.js'; -import { helpCommand } from '../ui/commands/helpCommand.js'; -import { clearCommand } from '../ui/commands/clearCommand.js'; -import { copyCommand } from '../ui/commands/copyCommand.js'; -import { corgiCommand } from '../ui/commands/corgiCommand.js'; -import { docsCommand } from '../ui/commands/docsCommand.js'; -import { mcpCommand } from '../ui/commands/mcpCommand.js'; -import { authCommand } from '../ui/commands/authCommand.js'; -import { themeCommand } from '../ui/commands/themeCommand.js'; -import { editorCommand } from '../ui/commands/editorCommand.js'; -import { chatCommand } from '../ui/commands/chatCommand.js'; -import { statsCommand } from '../ui/commands/statsCommand.js'; -import { privacyCommand } from '../ui/commands/privacyCommand.js'; -import { aboutCommand } from '../ui/commands/aboutCommand.js'; -import { extensionsCommand } from '../ui/commands/extensionsCommand.js'; -import { toolsCommand } from '../ui/commands/toolsCommand.js'; -import { compressCommand } from '../ui/commands/compressCommand.js'; -import { ideCommand } from '../ui/commands/ideCommand.js'; -import { bugCommand } from '../ui/commands/bugCommand.js'; -import { quitCommand } from '../ui/commands/quitCommand.js'; -import { restoreCommand } from '../ui/commands/restoreCommand.js'; - -const loadBuiltInCommands = async ( - config: Config | null, -): Promise => { - const allCommands = [ - aboutCommand, - authCommand, - bugCommand, - chatCommand, - clearCommand, - copyCommand, - compressCommand, - corgiCommand, - docsCommand, - editorCommand, - extensionsCommand, - helpCommand, - ideCommand(config), - mcpCommand, - memoryCommand, - privacyCommand, - quitCommand, - restoreCommand(config), - statsCommand, - themeCommand, - toolsCommand, - ]; - - return allCommands.filter( - (command): command is SlashCommand => command !== null, - ); -}; +import { ICommandLoader } from './types.js'; +/** + * Orchestrates the discovery and loading of all slash commands for the CLI. + * + * This service operates on a provider-based loader pattern. It is initialized + * with an array of `ICommandLoader` instances, each responsible for fetching + * commands from a specific source (e.g., built-in code, local files). + * + * The CommandService is responsible for invoking these loaders, aggregating their + * results, and resolving any name conflicts. This architecture allows the command + * system to be extended with new sources without modifying the service itself. + */ export class CommandService { - private commands: SlashCommand[] = []; + /** + * Private constructor to enforce the use of the async factory. + * @param commands A readonly array of the fully loaded and de-duplicated commands. + */ + private constructor(private readonly commands: readonly SlashCommand[]) {} - constructor( - private config: Config | null, - private commandLoader: ( - config: Config | null, - ) => Promise = loadBuiltInCommands, - ) { - // The constructor can be used for dependency injection in the future. + /** + * Asynchronously creates and initializes a new CommandService instance. + * + * This factory method orchestrates the entire command loading process. It + * runs all provided loaders in parallel, aggregates their results, handles + * name conflicts by letting the last-loaded command win, and then returns a + * fully constructed `CommandService` instance. + * + * @param loaders An array of objects that conform to the `ICommandLoader` + * interface. The order of loaders is significant: if multiple loaders + * provide a command with the same name, the command from the loader that + * appears later in the array will take precedence. + * @param signal An AbortSignal to cancel the loading process. + * @returns A promise that resolves to a new, fully initialized `CommandService` instance. + */ + static async create( + loaders: ICommandLoader[], + signal: AbortSignal, + ): Promise { + const results = await Promise.allSettled( + loaders.map((loader) => loader.loadCommands(signal)), + ); + + const allCommands: SlashCommand[] = []; + for (const result of results) { + if (result.status === 'fulfilled') { + allCommands.push(...result.value); + } else { + console.debug('A command loader failed:', result.reason); + } + } + + // De-duplicate commands using a Map. The last one found with a given name wins. + // This creates a natural override system based on the order of the loaders + // passed to the constructor. + const commandMap = new Map(); + for (const cmd of allCommands) { + commandMap.set(cmd.name, cmd); + } + + const finalCommands = Object.freeze(Array.from(commandMap.values())); + return new CommandService(finalCommands); } - async loadCommands(): Promise { - // For now, we only load the built-in commands. - // File-based and remote commands will be added later. - this.commands = await this.commandLoader(this.config); - } - - getCommands(): SlashCommand[] { + /** + * Retrieves the currently loaded and de-duplicated list of slash commands. + * + * This method is a safe accessor for the service's state. It returns a + * readonly array, preventing consumers from modifying the service's internal state. + * + * @returns A readonly, unified array of available `SlashCommand` objects. + */ + getCommands(): readonly SlashCommand[] { return this.commands; } } diff --git a/packages/cli/src/services/types.ts b/packages/cli/src/services/types.ts new file mode 100644 index 00000000..9d30e791 --- /dev/null +++ b/packages/cli/src/services/types.ts @@ -0,0 +1,24 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SlashCommand } from '../ui/commands/types.js'; + +/** + * Defines the contract for any class that can load and provide slash commands. + * This allows the CommandService to be extended with new command sources + * (e.g., file-based, remote APIs) without modification. + * + * Loaders should receive any necessary dependencies (like Config) via their + * constructor. + */ +export interface ICommandLoader { + /** + * Discovers and returns a list of slash commands from the loader's source. + * @param signal An AbortSignal to allow cancellation. + * @returns A promise that resolves to an array of SlashCommand objects. + */ + loadCommands(signal: AbortSignal): Promise; +} diff --git a/packages/cli/src/ui/commands/aboutCommand.ts b/packages/cli/src/ui/commands/aboutCommand.ts index 3cb8c2f6..18a82682 100644 --- a/packages/cli/src/ui/commands/aboutCommand.ts +++ b/packages/cli/src/ui/commands/aboutCommand.ts @@ -5,13 +5,14 @@ */ import { getCliVersion } from '../../utils/version.js'; -import { SlashCommand } from './types.js'; +import { CommandKind, SlashCommand } from './types.js'; import process from 'node:process'; import { MessageType, type HistoryItemAbout } from '../types.js'; export const aboutCommand: SlashCommand = { name: 'about', description: 'show version info', + kind: CommandKind.BUILT_IN, action: async (context) => { const osVersion = process.platform; let sandboxEnv = 'no sandbox'; diff --git a/packages/cli/src/ui/commands/authCommand.ts b/packages/cli/src/ui/commands/authCommand.ts index 29bd2c9d..8e78cf86 100644 --- a/packages/cli/src/ui/commands/authCommand.ts +++ b/packages/cli/src/ui/commands/authCommand.ts @@ -4,11 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { OpenDialogActionReturn, SlashCommand } from './types.js'; +import { CommandKind, OpenDialogActionReturn, SlashCommand } from './types.js'; export const authCommand: SlashCommand = { name: 'auth', description: 'change the auth method', + kind: CommandKind.BUILT_IN, action: (_context, _args): OpenDialogActionReturn => ({ type: 'dialog', dialog: 'auth', diff --git a/packages/cli/src/ui/commands/bugCommand.ts b/packages/cli/src/ui/commands/bugCommand.ts index c1b99db9..667276ab 100644 --- a/packages/cli/src/ui/commands/bugCommand.ts +++ b/packages/cli/src/ui/commands/bugCommand.ts @@ -6,7 +6,11 @@ import open from 'open'; import process from 'node:process'; -import { type CommandContext, type SlashCommand } from './types.js'; +import { + type CommandContext, + type SlashCommand, + CommandKind, +} from './types.js'; import { MessageType } from '../types.js'; import { GIT_COMMIT_INFO } from '../../generated/git-commit.js'; import { formatMemoryUsage } from '../utils/formatters.js'; @@ -15,6 +19,7 @@ import { getCliVersion } from '../../utils/version.js'; export const bugCommand: SlashCommand = { name: 'bug', description: 'submit a bug report', + kind: CommandKind.BUILT_IN, action: async (context: CommandContext, args?: string): Promise => { const bugDescription = (args || '').trim(); const { config } = context.services; diff --git a/packages/cli/src/ui/commands/chatCommand.ts b/packages/cli/src/ui/commands/chatCommand.ts index fd56afbd..2f669481 100644 --- a/packages/cli/src/ui/commands/chatCommand.ts +++ b/packages/cli/src/ui/commands/chatCommand.ts @@ -5,7 +5,12 @@ */ import * as fsPromises from 'fs/promises'; -import { CommandContext, SlashCommand, MessageActionReturn } from './types.js'; +import { + CommandContext, + SlashCommand, + MessageActionReturn, + CommandKind, +} from './types.js'; import path from 'path'; import { HistoryItemWithoutId, MessageType } from '../types.js'; @@ -54,6 +59,7 @@ const getSavedChatTags = async ( const listCommand: SlashCommand = { name: 'list', description: 'List saved conversation checkpoints', + kind: CommandKind.BUILT_IN, action: async (context): Promise => { const chatDetails = await getSavedChatTags(context, false); if (chatDetails.length === 0) { @@ -81,6 +87,7 @@ const saveCommand: SlashCommand = { name: 'save', description: 'Save the current conversation as a checkpoint. Usage: /chat save ', + kind: CommandKind.BUILT_IN, action: async (context, args): Promise => { const tag = args.trim(); if (!tag) { @@ -122,9 +129,10 @@ const saveCommand: SlashCommand = { const resumeCommand: SlashCommand = { name: 'resume', - altName: 'load', + altNames: ['load'], description: 'Resume a conversation from a checkpoint. Usage: /chat resume ', + kind: CommandKind.BUILT_IN, action: async (context, args) => { const tag = args.trim(); if (!tag) { @@ -193,5 +201,6 @@ const resumeCommand: SlashCommand = { export const chatCommand: SlashCommand = { name: 'chat', description: 'Manage conversation history.', + kind: CommandKind.BUILT_IN, subCommands: [listCommand, saveCommand, resumeCommand], }; diff --git a/packages/cli/src/ui/commands/clearCommand.ts b/packages/cli/src/ui/commands/clearCommand.ts index 1c409359..a2a1c13a 100644 --- a/packages/cli/src/ui/commands/clearCommand.ts +++ b/packages/cli/src/ui/commands/clearCommand.ts @@ -5,11 +5,12 @@ */ import { uiTelemetryService } from '@google/gemini-cli-core'; -import { SlashCommand } from './types.js'; +import { CommandKind, SlashCommand } from './types.js'; export const clearCommand: SlashCommand = { name: 'clear', description: 'clear the screen and conversation history', + kind: CommandKind.BUILT_IN, action: async (context, _args) => { const geminiClient = context.services.config?.getGeminiClient(); diff --git a/packages/cli/src/ui/commands/compressCommand.ts b/packages/cli/src/ui/commands/compressCommand.ts index c3dfdf37..792e8b5b 100644 --- a/packages/cli/src/ui/commands/compressCommand.ts +++ b/packages/cli/src/ui/commands/compressCommand.ts @@ -5,12 +5,13 @@ */ import { HistoryItemCompression, MessageType } from '../types.js'; -import { SlashCommand } from './types.js'; +import { CommandKind, SlashCommand } from './types.js'; export const compressCommand: SlashCommand = { name: 'compress', - altName: 'summarize', + altNames: ['summarize'], description: 'Compresses the context by replacing it with a summary.', + kind: CommandKind.BUILT_IN, action: async (context) => { const { ui } = context; if (ui.pendingItem) { diff --git a/packages/cli/src/ui/commands/copyCommand.ts b/packages/cli/src/ui/commands/copyCommand.ts index 5714b5ab..bd330faa 100644 --- a/packages/cli/src/ui/commands/copyCommand.ts +++ b/packages/cli/src/ui/commands/copyCommand.ts @@ -5,11 +5,16 @@ */ import { copyToClipboard } from '../utils/commandUtils.js'; -import { SlashCommand, SlashCommandActionReturn } from './types.js'; +import { + CommandKind, + SlashCommand, + SlashCommandActionReturn, +} from './types.js'; export const copyCommand: SlashCommand = { name: 'copy', description: 'Copy the last result or code snippet to clipboard', + kind: CommandKind.BUILT_IN, action: async (context, _args): Promise => { const chat = await context.services.config?.getGeminiClient()?.getChat(); const history = chat?.getHistory(); diff --git a/packages/cli/src/ui/commands/corgiCommand.ts b/packages/cli/src/ui/commands/corgiCommand.ts index 290c071e..cb3ecd1c 100644 --- a/packages/cli/src/ui/commands/corgiCommand.ts +++ b/packages/cli/src/ui/commands/corgiCommand.ts @@ -4,11 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { type SlashCommand } from './types.js'; +import { CommandKind, type SlashCommand } from './types.js'; export const corgiCommand: SlashCommand = { name: 'corgi', description: 'Toggles corgi mode.', + kind: CommandKind.BUILT_IN, action: (context, _args) => { context.ui.toggleCorgiMode(); }, diff --git a/packages/cli/src/ui/commands/docsCommand.ts b/packages/cli/src/ui/commands/docsCommand.ts index e53a4a80..922b236a 100644 --- a/packages/cli/src/ui/commands/docsCommand.ts +++ b/packages/cli/src/ui/commands/docsCommand.ts @@ -6,12 +6,17 @@ import open from 'open'; import process from 'node:process'; -import { type CommandContext, type SlashCommand } from './types.js'; +import { + type CommandContext, + type SlashCommand, + CommandKind, +} from './types.js'; import { MessageType } from '../types.js'; export const docsCommand: SlashCommand = { name: 'docs', description: 'open full Gemini CLI documentation in your browser', + kind: CommandKind.BUILT_IN, action: async (context: CommandContext): Promise => { const docsUrl = 'https://goo.gle/gemini-cli-docs'; diff --git a/packages/cli/src/ui/commands/editorCommand.ts b/packages/cli/src/ui/commands/editorCommand.ts index dbfafa51..5b5c4c5d 100644 --- a/packages/cli/src/ui/commands/editorCommand.ts +++ b/packages/cli/src/ui/commands/editorCommand.ts @@ -4,11 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { type OpenDialogActionReturn, type SlashCommand } from './types.js'; +import { + CommandKind, + type OpenDialogActionReturn, + type SlashCommand, +} from './types.js'; export const editorCommand: SlashCommand = { name: 'editor', description: 'set external editor preference', + kind: CommandKind.BUILT_IN, action: (): OpenDialogActionReturn => ({ type: 'dialog', dialog: 'editor', diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index 09241e5f..ea9f9a4f 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -4,12 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { type CommandContext, type SlashCommand } from './types.js'; +import { + type CommandContext, + type SlashCommand, + CommandKind, +} from './types.js'; import { MessageType } from '../types.js'; export const extensionsCommand: SlashCommand = { name: 'extensions', description: 'list active extensions', + kind: CommandKind.BUILT_IN, action: async (context: CommandContext): Promise => { const activeExtensions = context.services.config ?.getExtensions() diff --git a/packages/cli/src/ui/commands/helpCommand.test.ts b/packages/cli/src/ui/commands/helpCommand.test.ts index a6b19c05..b0441106 100644 --- a/packages/cli/src/ui/commands/helpCommand.test.ts +++ b/packages/cli/src/ui/commands/helpCommand.test.ts @@ -32,9 +32,9 @@ describe('helpCommand', () => { }); it("should also be triggered by its alternative name '?'", () => { - // This test is more conceptual. The routing of altName to the command + // This test is more conceptual. The routing of altNames to the command // is handled by the slash command processor, but we can assert the - // altName is correctly defined on the command object itself. - expect(helpCommand.altName).toBe('?'); + // altNames is correctly defined on the command object itself. + expect(helpCommand.altNames).toContain('?'); }); }); diff --git a/packages/cli/src/ui/commands/helpCommand.ts b/packages/cli/src/ui/commands/helpCommand.ts index 82d0d536..03c64615 100644 --- a/packages/cli/src/ui/commands/helpCommand.ts +++ b/packages/cli/src/ui/commands/helpCommand.ts @@ -4,12 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { OpenDialogActionReturn, SlashCommand } from './types.js'; +import { CommandKind, OpenDialogActionReturn, SlashCommand } from './types.js'; export const helpCommand: SlashCommand = { name: 'help', - altName: '?', + altNames: ['?'], description: 'for help on gemini-cli', + kind: CommandKind.BUILT_IN, action: (_context, _args): OpenDialogActionReturn => { console.debug('Opening help UI ...'); return { diff --git a/packages/cli/src/ui/commands/ideCommand.ts b/packages/cli/src/ui/commands/ideCommand.ts index 0251e619..6fc4f50b 100644 --- a/packages/cli/src/ui/commands/ideCommand.ts +++ b/packages/cli/src/ui/commands/ideCommand.ts @@ -17,6 +17,7 @@ import { CommandContext, SlashCommand, SlashCommandActionReturn, + CommandKind, } from './types.js'; import * as child_process from 'child_process'; import * as process from 'process'; @@ -48,10 +49,12 @@ export const ideCommand = (config: Config | null): SlashCommand | null => { return { name: 'ide', description: 'manage IDE integration', + kind: CommandKind.BUILT_IN, subCommands: [ { name: 'status', description: 'check status of IDE integration', + kind: CommandKind.BUILT_IN, action: (_context: CommandContext): SlashCommandActionReturn => { const status = getMCPServerStatus(IDE_SERVER_NAME); const discoveryState = getMCPDiscoveryState(); @@ -89,6 +92,7 @@ export const ideCommand = (config: Config | null): SlashCommand | null => { { name: 'install', description: 'install required VS Code companion extension', + kind: CommandKind.BUILT_IN, action: async (context) => { if (!isVSCodeInstalled()) { context.ui.addItem( diff --git a/packages/cli/src/ui/commands/mcpCommand.ts b/packages/cli/src/ui/commands/mcpCommand.ts index 891227b0..373f1ca5 100644 --- a/packages/cli/src/ui/commands/mcpCommand.ts +++ b/packages/cli/src/ui/commands/mcpCommand.ts @@ -8,6 +8,7 @@ import { SlashCommand, SlashCommandActionReturn, CommandContext, + CommandKind, } from './types.js'; import { DiscoveredMCPTool, @@ -229,6 +230,7 @@ const getMcpStatus = async ( export const mcpCommand: SlashCommand = { name: 'mcp', description: 'list configured MCP servers and tools', + kind: CommandKind.BUILT_IN, action: async (context: CommandContext, args: string) => { const lowerCaseArgs = args.toLowerCase().split(/\s+/).filter(Boolean); diff --git a/packages/cli/src/ui/commands/memoryCommand.ts b/packages/cli/src/ui/commands/memoryCommand.ts index 18ca96bb..afa43031 100644 --- a/packages/cli/src/ui/commands/memoryCommand.ts +++ b/packages/cli/src/ui/commands/memoryCommand.ts @@ -6,15 +6,21 @@ import { getErrorMessage } from '@google/gemini-cli-core'; import { MessageType } from '../types.js'; -import { SlashCommand, SlashCommandActionReturn } from './types.js'; +import { + CommandKind, + SlashCommand, + SlashCommandActionReturn, +} from './types.js'; export const memoryCommand: SlashCommand = { name: 'memory', description: 'Commands for interacting with memory.', + kind: CommandKind.BUILT_IN, subCommands: [ { name: 'show', description: 'Show the current memory contents.', + kind: CommandKind.BUILT_IN, action: async (context) => { const memoryContent = context.services.config?.getUserMemory() || ''; const fileCount = context.services.config?.getGeminiMdFileCount() || 0; @@ -36,6 +42,7 @@ export const memoryCommand: SlashCommand = { { name: 'add', description: 'Add content to the memory.', + kind: CommandKind.BUILT_IN, action: (context, args): SlashCommandActionReturn | void => { if (!args || args.trim() === '') { return { @@ -63,6 +70,7 @@ export const memoryCommand: SlashCommand = { { name: 'refresh', description: 'Refresh the memory from the source.', + kind: CommandKind.BUILT_IN, action: async (context) => { context.ui.addItem( { diff --git a/packages/cli/src/ui/commands/privacyCommand.ts b/packages/cli/src/ui/commands/privacyCommand.ts index f239158c..ef9d08a0 100644 --- a/packages/cli/src/ui/commands/privacyCommand.ts +++ b/packages/cli/src/ui/commands/privacyCommand.ts @@ -4,11 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { OpenDialogActionReturn, SlashCommand } from './types.js'; +import { CommandKind, OpenDialogActionReturn, SlashCommand } from './types.js'; export const privacyCommand: SlashCommand = { name: 'privacy', description: 'display the privacy notice', + kind: CommandKind.BUILT_IN, action: (): OpenDialogActionReturn => ({ type: 'dialog', dialog: 'privacy', diff --git a/packages/cli/src/ui/commands/quitCommand.ts b/packages/cli/src/ui/commands/quitCommand.ts index 48daf8c2..36f15c71 100644 --- a/packages/cli/src/ui/commands/quitCommand.ts +++ b/packages/cli/src/ui/commands/quitCommand.ts @@ -5,12 +5,13 @@ */ import { formatDuration } from '../utils/formatters.js'; -import { type SlashCommand } from './types.js'; +import { CommandKind, type SlashCommand } from './types.js'; export const quitCommand: SlashCommand = { name: 'quit', - altName: 'exit', + altNames: ['exit'], description: 'exit the cli', + kind: CommandKind.BUILT_IN, action: (context) => { const now = Date.now(); const { sessionStartTime } = context.session.stats; diff --git a/packages/cli/src/ui/commands/restoreCommand.ts b/packages/cli/src/ui/commands/restoreCommand.ts index 3d744189..84259288 100644 --- a/packages/cli/src/ui/commands/restoreCommand.ts +++ b/packages/cli/src/ui/commands/restoreCommand.ts @@ -10,6 +10,7 @@ import { type CommandContext, type SlashCommand, type SlashCommandActionReturn, + CommandKind, } from './types.js'; import { Config } from '@google/gemini-cli-core'; @@ -149,6 +150,7 @@ export const restoreCommand = (config: Config | null): SlashCommand | null => { name: 'restore', description: 'Restore a tool call. This will reset the conversation and file history to the state it was in when the tool call was suggested', + kind: CommandKind.BUILT_IN, action: restoreAction, completion, }; diff --git a/packages/cli/src/ui/commands/statsCommand.ts b/packages/cli/src/ui/commands/statsCommand.ts index 87e902d4..e9e69756 100644 --- a/packages/cli/src/ui/commands/statsCommand.ts +++ b/packages/cli/src/ui/commands/statsCommand.ts @@ -6,12 +6,17 @@ import { MessageType, HistoryItemStats } from '../types.js'; import { formatDuration } from '../utils/formatters.js'; -import { type CommandContext, type SlashCommand } from './types.js'; +import { + type CommandContext, + type SlashCommand, + CommandKind, +} from './types.js'; export const statsCommand: SlashCommand = { name: 'stats', - altName: 'usage', + altNames: ['usage'], description: 'check session stats. Usage: /stats [model|tools]', + kind: CommandKind.BUILT_IN, action: (context: CommandContext) => { const now = new Date(); const { sessionStartTime } = context.session.stats; @@ -38,6 +43,7 @@ export const statsCommand: SlashCommand = { { name: 'model', description: 'Show model-specific usage statistics.', + kind: CommandKind.BUILT_IN, action: (context: CommandContext) => { context.ui.addItem( { @@ -50,6 +56,7 @@ export const statsCommand: SlashCommand = { { name: 'tools', description: 'Show tool-specific usage statistics.', + kind: CommandKind.BUILT_IN, action: (context: CommandContext) => { context.ui.addItem( { diff --git a/packages/cli/src/ui/commands/themeCommand.ts b/packages/cli/src/ui/commands/themeCommand.ts index 29e9a491..755d59d9 100644 --- a/packages/cli/src/ui/commands/themeCommand.ts +++ b/packages/cli/src/ui/commands/themeCommand.ts @@ -4,11 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { OpenDialogActionReturn, SlashCommand } from './types.js'; +import { CommandKind, OpenDialogActionReturn, SlashCommand } from './types.js'; export const themeCommand: SlashCommand = { name: 'theme', description: 'change the theme', + kind: CommandKind.BUILT_IN, action: (_context, _args): OpenDialogActionReturn => ({ type: 'dialog', dialog: 'theme', diff --git a/packages/cli/src/ui/commands/toolsCommand.ts b/packages/cli/src/ui/commands/toolsCommand.ts index f65edd07..e993bab3 100644 --- a/packages/cli/src/ui/commands/toolsCommand.ts +++ b/packages/cli/src/ui/commands/toolsCommand.ts @@ -4,12 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { type CommandContext, type SlashCommand } from './types.js'; +import { + type CommandContext, + type SlashCommand, + CommandKind, +} from './types.js'; import { MessageType } from '../types.js'; export const toolsCommand: SlashCommand = { name: 'tools', description: 'list available Gemini CLI tools', + kind: CommandKind.BUILT_IN, action: async (context: CommandContext, args?: string): Promise => { const subCommand = args?.trim(); diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index 3e269cbf..3ffadf83 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -106,11 +106,18 @@ export type SlashCommandActionReturn = | OpenDialogActionReturn | LoadHistoryActionReturn; +export enum CommandKind { + BUILT_IN = 'built-in', + FILE = 'file', +} + // The standardized contract for any command in the system. export interface SlashCommand { name: string; - altName?: string; - description?: string; + altNames?: string[]; + description: string; + + kind: CommandKind; // The action to run. Optional for parent commands that only group sub-commands. action?: ( diff --git a/packages/cli/src/ui/components/Help.tsx b/packages/cli/src/ui/components/Help.tsx index c51867af..ecad9b5e 100644 --- a/packages/cli/src/ui/components/Help.tsx +++ b/packages/cli/src/ui/components/Help.tsx @@ -10,7 +10,7 @@ import { Colors } from '../colors.js'; import { SlashCommand } from '../commands/types.js'; interface Help { - commands: SlashCommand[]; + commands: readonly SlashCommand[]; } export const Help: React.FC = ({ commands }) => ( diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 6b201901..886a6235 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -451,13 +451,13 @@ describe('InputPrompt', () => { unmount(); }); - it('should complete a command based on its altName', async () => { - // Add a command with an altName to our mock for this test + it('should complete a command based on its altNames', async () => { + // Add a command with an altNames to our mock for this test props.slashCommands.push({ name: 'help', - altName: '?', + altNames: ['?'], description: '...', - }); + } as SlashCommand); mockedUseCompletion.mockReturnValue({ ...mockCompletion, diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 46326431..b7c53196 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -32,7 +32,7 @@ export interface InputPromptProps { userMessages: readonly string[]; onClearScreen: () => void; config: Config; - slashCommands: SlashCommand[]; + slashCommands: readonly SlashCommand[]; commandContext: CommandContext; placeholder?: string; focus?: boolean; @@ -180,18 +180,20 @@ export const InputPrompt: React.FC = ({ // If there's no trailing space, we need to check if the current query // is already a complete path to a parent command. if (!hasTrailingSpace) { - let currentLevel: SlashCommand[] | undefined = slashCommands; + let currentLevel: readonly SlashCommand[] | undefined = slashCommands; for (let i = 0; i < parts.length; i++) { const part = parts[i]; const found: SlashCommand | undefined = currentLevel?.find( - (cmd) => cmd.name === part || cmd.altName === part, + (cmd) => cmd.name === part || cmd.altNames?.includes(part), ); if (found) { if (i === parts.length - 1 && found.subCommands) { isParentPath = true; } - currentLevel = found.subCommands; + currentLevel = found.subCommands as + | readonly SlashCommand[] + | undefined; } else { // Path is invalid, so it can't be a parent path. currentLevel = undefined; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 4920b088..32a6810e 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -11,413 +11,169 @@ const { mockProcessExit } = vi.hoisted(() => ({ vi.mock('node:process', () => ({ default: { exit: mockProcessExit, - cwd: vi.fn(() => '/mock/cwd'), - get env() { - return process.env; - }, - platform: 'test-platform', - version: 'test-node-version', - memoryUsage: vi.fn(() => ({ - rss: 12345678, - heapTotal: 23456789, - heapUsed: 10234567, - external: 1234567, - arrayBuffers: 123456, - })), }, - exit: mockProcessExit, - cwd: vi.fn(() => '/mock/cwd'), - get env() { - return process.env; - }, - platform: 'test-platform', - version: 'test-node-version', - memoryUsage: vi.fn(() => ({ - rss: 12345678, - heapTotal: 23456789, - heapUsed: 10234567, - external: 1234567, - arrayBuffers: 123456, +})); + +const mockLoadCommands = vi.fn(); +vi.mock('../../services/BuiltinCommandLoader.js', () => ({ + BuiltinCommandLoader: vi.fn().mockImplementation(() => ({ + loadCommands: mockLoadCommands, })), })); -vi.mock('node:fs/promises', () => ({ - readFile: vi.fn(), - writeFile: vi.fn(), - mkdir: vi.fn(), -})); - -const mockGetCliVersionFn = vi.fn(() => Promise.resolve('0.1.0')); -vi.mock('../../utils/version.js', () => ({ - getCliVersion: (...args: []) => mockGetCliVersionFn(...args), -})); - -import { act, renderHook } from '@testing-library/react'; -import { vi, describe, it, expect, beforeEach, beforeAll, Mock } from 'vitest'; -import open from 'open'; -import { useSlashCommandProcessor } from './slashCommandProcessor.js'; -import { SlashCommandProcessorResult } from '../types.js'; -import { Config, GeminiClient } from '@google/gemini-cli-core'; -import { useSessionStats } from '../contexts/SessionContext.js'; -import { LoadedSettings } from '../../config/settings.js'; -import * as ShowMemoryCommandModule from './useShowMemoryCommand.js'; -import { CommandService } from '../../services/CommandService.js'; -import { SlashCommand } from '../commands/types.js'; - vi.mock('../contexts/SessionContext.js', () => ({ - useSessionStats: vi.fn(), + useSessionStats: vi.fn(() => ({ stats: {} })), })); -vi.mock('../../services/CommandService.js'); - -vi.mock('./useShowMemoryCommand.js', () => ({ - SHOW_MEMORY_COMMAND_NAME: '/memory show', - createShowMemoryAction: vi.fn(() => vi.fn()), -})); - -vi.mock('open', () => ({ - default: vi.fn(), -})); - -vi.mock('@google/gemini-cli-core', async (importOriginal) => { - const actual = - await importOriginal(); - return { - ...actual, - }; -}); +import { act, renderHook, waitFor } from '@testing-library/react'; +import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest'; +import { useSlashCommandProcessor } from './slashCommandProcessor.js'; +import { SlashCommand } from '../commands/types.js'; +import { Config } from '@google/gemini-cli-core'; +import { LoadedSettings } from '../../config/settings.js'; +import { MessageType } from '../types.js'; +import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js'; describe('useSlashCommandProcessor', () => { - let mockAddItem: ReturnType; - let mockClearItems: ReturnType; - let mockLoadHistory: ReturnType; - let mockRefreshStatic: ReturnType; - let mockSetShowHelp: ReturnType; - let mockOnDebugMessage: ReturnType; - let mockOpenThemeDialog: ReturnType; - let mockOpenAuthDialog: ReturnType; - let mockOpenEditorDialog: ReturnType; - let mockSetQuittingMessages: ReturnType; - let mockTryCompressChat: ReturnType; - let mockGeminiClient: GeminiClient; - let mockConfig: Config; - let mockCorgiMode: ReturnType; - const mockUseSessionStats = useSessionStats as Mock; + const mockAddItem = vi.fn(); + const mockClearItems = vi.fn(); + const mockLoadHistory = vi.fn(); + const mockSetShowHelp = vi.fn(); + const mockOpenAuthDialog = vi.fn(); + const mockSetQuittingMessages = vi.fn(); + + const mockConfig = { + getProjectRoot: () => '/mock/cwd', + getSessionId: () => 'test-session', + getGeminiClient: () => ({ + setHistory: vi.fn().mockResolvedValue(undefined), + }), + } as unknown as Config; + + const mockSettings = {} as LoadedSettings; beforeEach(() => { vi.clearAllMocks(); - - mockAddItem = vi.fn(); - mockClearItems = vi.fn(); - mockLoadHistory = vi.fn(); - mockRefreshStatic = vi.fn(); - mockSetShowHelp = vi.fn(); - mockOnDebugMessage = vi.fn(); - mockOpenThemeDialog = vi.fn(); - mockOpenAuthDialog = vi.fn(); - mockOpenEditorDialog = vi.fn(); - mockSetQuittingMessages = vi.fn(); - mockTryCompressChat = vi.fn(); - mockGeminiClient = { - tryCompressChat: mockTryCompressChat, - } as unknown as GeminiClient; - mockConfig = { - getDebugMode: vi.fn(() => false), - getGeminiClient: () => mockGeminiClient, - getSandbox: vi.fn(() => 'test-sandbox'), - getModel: vi.fn(() => 'test-model'), - getProjectRoot: vi.fn(() => '/test/dir'), - getCheckpointingEnabled: vi.fn(() => true), - getBugCommand: vi.fn(() => undefined), - getSessionId: vi.fn(() => 'test-session-id'), - getIdeMode: vi.fn(() => false), - } as unknown as Config; - mockCorgiMode = vi.fn(); - mockUseSessionStats.mockReturnValue({ - stats: { - sessionStartTime: new Date('2025-01-01T00:00:00.000Z'), - cumulative: { - promptCount: 0, - promptTokenCount: 0, - candidatesTokenCount: 0, - totalTokenCount: 0, - cachedContentTokenCount: 0, - toolUsePromptTokenCount: 0, - thoughtsTokenCount: 0, - }, - }, - }); - - (open as Mock).mockClear(); - mockProcessExit.mockClear(); - (ShowMemoryCommandModule.createShowMemoryAction as Mock).mockClear(); - process.env = { ...globalThis.process.env }; + (vi.mocked(BuiltinCommandLoader) as Mock).mockClear(); + mockLoadCommands.mockResolvedValue([]); }); - const getProcessorHook = () => { - const settings = { - merged: { - contextFileName: 'GEMINI.md', - }, - } as unknown as LoadedSettings; - return renderHook(() => + const setupProcessorHook = (commands: SlashCommand[] = []) => { + mockLoadCommands.mockResolvedValue(Object.freeze(commands)); + const { result } = renderHook(() => useSlashCommandProcessor( mockConfig, - settings, + mockSettings, mockAddItem, mockClearItems, mockLoadHistory, - mockRefreshStatic, + vi.fn(), // refreshStatic mockSetShowHelp, - mockOnDebugMessage, - mockOpenThemeDialog, + vi.fn(), // onDebugMessage + vi.fn(), // openThemeDialog mockOpenAuthDialog, - mockOpenEditorDialog, - mockCorgiMode, + vi.fn(), // openEditorDialog + vi.fn(), // toggleCorgiMode mockSetQuittingMessages, - vi.fn(), // mockOpenPrivacyNotice + vi.fn(), // openPrivacyNotice ), ); + + return result; }; - describe('Command Processing', () => { - let ActualCommandService: typeof CommandService; - - beforeAll(async () => { - const actual = (await vi.importActual( - '../../services/CommandService.js', - )) as { CommandService: typeof CommandService }; - ActualCommandService = actual.CommandService; + describe('Initialization and Command Loading', () => { + it('should initialize CommandService with BuiltinCommandLoader', () => { + setupProcessorHook(); + expect(BuiltinCommandLoader).toHaveBeenCalledTimes(1); + expect(BuiltinCommandLoader).toHaveBeenCalledWith(mockConfig); }); - beforeEach(() => { - vi.clearAllMocks(); - }); + it('should call loadCommands and populate state after mounting', async () => { + const testCommand: SlashCommand = { + name: 'test', + description: 'a test command', + kind: 'built-in', + }; + const result = setupProcessorHook([testCommand]); - it('should execute a registered command', async () => { - const mockAction = vi.fn(); - const newCommand: SlashCommand = { name: 'test', action: mockAction }; - const mockLoader = async () => [newCommand]; - - // We create the instance outside the mock implementation. - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - - // This mock ensures the hook uses our pre-configured instance. - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - - await vi.waitFor(() => { - // We check that the `slashCommands` array, which is the public API - // of our hook, eventually contains the command we injected. - expect( - result.current.slashCommands.some((c) => c.name === 'test'), - ).toBe(true); + await waitFor(() => { + expect(result.current.slashCommands).toHaveLength(1); }); - let commandResult: SlashCommandProcessorResult | false = false; + expect(result.current.slashCommands[0]?.name).toBe('test'); + expect(mockLoadCommands).toHaveBeenCalledTimes(1); + }); + + it('should provide an immutable array of commands to consumers', async () => { + const testCommand: SlashCommand = { + name: 'test', + description: 'a test command', + kind: 'built-in', + }; + const result = setupProcessorHook([testCommand]); + + await waitFor(() => { + expect(result.current.slashCommands).toHaveLength(1); + }); + + const commands = result.current.slashCommands; + + expect(() => { + // @ts-expect-error - We are intentionally testing a violation of the readonly type. + commands.push({ + name: 'rogue', + description: 'a rogue command', + kind: 'built-in', + }); + }).toThrow(TypeError); + }); + }); + + describe('Command Execution Logic', () => { + it('should display an error for an unknown command', async () => { + const result = setupProcessorHook(); + await waitFor(() => expect(result.current.slashCommands).toBeDefined()); + await act(async () => { - commandResult = await result.current.handleSlashCommand('/test'); + await result.current.handleSlashCommand('/nonexistent'); }); - expect(mockAction).toHaveBeenCalledTimes(1); - expect(commandResult).toEqual({ type: 'handled' }); - }); - - it('should return "schedule_tool" for a command returning a tool action', async () => { - const mockAction = vi.fn().mockResolvedValue({ - type: 'tool', - toolName: 'my_tool', - toolArgs: { arg1: 'value1' }, - }); - const newCommand: SlashCommand = { name: 'test', action: mockAction }; - const mockLoader = async () => [newCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'test'), - ).toBe(true); - }); - - const commandResult = await result.current.handleSlashCommand('/test'); - - expect(mockAction).toHaveBeenCalledTimes(1); - expect(commandResult).toEqual({ - type: 'schedule_tool', - toolName: 'my_tool', - toolArgs: { arg1: 'value1' }, - }); - }); - - it('should return "handled" for a command returning a message action', async () => { - const mockAction = vi.fn().mockResolvedValue({ - type: 'message', - messageType: 'info', - content: 'This is a message', - }); - const newCommand: SlashCommand = { name: 'test', action: mockAction }; - const mockLoader = async () => [newCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'test'), - ).toBe(true); - }); - - const commandResult = await result.current.handleSlashCommand('/test'); - - expect(mockAction).toHaveBeenCalledTimes(1); - expect(mockAddItem).toHaveBeenCalledWith( + // Expect 2 calls: one for the user's input, one for the error message. + expect(mockAddItem).toHaveBeenCalledTimes(2); + expect(mockAddItem).toHaveBeenLastCalledWith( expect.objectContaining({ - type: 'info', - text: 'This is a message', + type: MessageType.ERROR, + text: 'Unknown command: /nonexistent', }), expect.any(Number), ); - expect(commandResult).toEqual({ type: 'handled' }); }); - it('should return "handled" for a command returning a dialog action', async () => { - const mockAction = vi.fn().mockResolvedValue({ - type: 'dialog', - dialog: 'help', - }); - const newCommand: SlashCommand = { name: 'test', action: mockAction }; - const mockLoader = async () => [newCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'test'), - ).toBe(true); - }); - - const commandResult = await result.current.handleSlashCommand('/test'); - - expect(mockAction).toHaveBeenCalledTimes(1); - expect(mockSetShowHelp).toHaveBeenCalledWith(true); - expect(commandResult).toEqual({ type: 'handled' }); - }); - - it('should open the auth dialog for a command returning an auth dialog action', async () => { - const mockAction = vi.fn().mockResolvedValue({ - type: 'dialog', - dialog: 'auth', - }); - const newAuthCommand: SlashCommand = { name: 'auth', action: mockAction }; - - const mockLoader = async () => [newAuthCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'auth'), - ).toBe(true); - }); - - const commandResult = await result.current.handleSlashCommand('/auth'); - - expect(mockAction).toHaveBeenCalledTimes(1); - expect(mockOpenAuthDialog).toHaveBeenCalledWith(); - expect(commandResult).toEqual({ type: 'handled' }); - }); - - it('should open the theme dialog for a command returning a theme dialog action', async () => { - const mockAction = vi.fn().mockResolvedValue({ - type: 'dialog', - dialog: 'theme', - }); - const newCommand: SlashCommand = { name: 'test', action: mockAction }; - const mockLoader = async () => [newCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'test'), - ).toBe(true); - }); - - const commandResult = await result.current.handleSlashCommand('/test'); - - expect(mockAction).toHaveBeenCalledTimes(1); - expect(mockOpenThemeDialog).toHaveBeenCalledWith(); - expect(commandResult).toEqual({ type: 'handled' }); - }); - - it('should show help for a parent command with no action', async () => { + it('should display help for a parent command invoked without a subcommand', async () => { const parentCommand: SlashCommand = { name: 'parent', + description: 'a parent command', + kind: 'built-in', subCommands: [ - { name: 'child', description: 'A child.', action: vi.fn() }, + { + name: 'child1', + description: 'First child.', + kind: 'built-in', + }, ], }; - - const mockLoader = async () => [parentCommand]; - const commandServiceInstance = new ActualCommandService( - mockConfig, - mockLoader, - ); - vi.mocked(CommandService).mockImplementation( - () => commandServiceInstance, - ); - - const { result } = getProcessorHook(); - - await vi.waitFor(() => { - expect( - result.current.slashCommands.some((c) => c.name === 'parent'), - ).toBe(true); - }); + const result = setupProcessorHook([parentCommand]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { await result.current.handleSlashCommand('/parent'); }); - expect(mockAddItem).toHaveBeenCalledWith( + expect(mockAddItem).toHaveBeenCalledTimes(2); + expect(mockAddItem).toHaveBeenLastCalledWith( expect.objectContaining({ - type: 'info', + type: MessageType.INFO, text: expect.stringContaining( "Command '/parent' requires a subcommand.", ), @@ -425,5 +181,218 @@ describe('useSlashCommandProcessor', () => { expect.any(Number), ); }); + + it('should correctly find and execute a nested subcommand', async () => { + const childAction = vi.fn(); + const parentCommand: SlashCommand = { + name: 'parent', + description: 'a parent command', + kind: 'built-in', + subCommands: [ + { + name: 'child', + description: 'a child command', + kind: 'built-in', + action: childAction, + }, + ], + }; + const result = setupProcessorHook([parentCommand]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + await result.current.handleSlashCommand('/parent child with args'); + }); + + expect(childAction).toHaveBeenCalledTimes(1); + + expect(childAction).toHaveBeenCalledWith( + expect.objectContaining({ + services: expect.objectContaining({ + config: mockConfig, + }), + ui: expect.objectContaining({ + addItem: mockAddItem, + }), + }), + 'with args', + ); + }); + }); + + describe('Action Result Handling', () => { + it('should handle "dialog: help" action', async () => { + const command: SlashCommand = { + name: 'helpcmd', + description: 'a help command', + kind: 'built-in', + action: vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'help' }), + }; + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + await result.current.handleSlashCommand('/helpcmd'); + }); + + expect(mockSetShowHelp).toHaveBeenCalledWith(true); + }); + + it('should handle "load_history" action', async () => { + const command: SlashCommand = { + name: 'load', + description: 'a load command', + kind: 'built-in', + action: vi.fn().mockResolvedValue({ + type: 'load_history', + history: [{ type: MessageType.USER, text: 'old prompt' }], + clientHistory: [{ role: 'user', parts: [{ text: 'old prompt' }] }], + }), + }; + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + await result.current.handleSlashCommand('/load'); + }); + + expect(mockClearItems).toHaveBeenCalledTimes(1); + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ type: 'user', text: 'old prompt' }), + expect.any(Number), + ); + }); + + describe('with fake timers', () => { + // This test needs to let the async `waitFor` complete with REAL timers + // before switching to FAKE timers to test setTimeout. + it('should handle a "quit" action', async () => { + const quitAction = vi + .fn() + .mockResolvedValue({ type: 'quit', messages: [] }); + const command: SlashCommand = { + name: 'exit', + description: 'an exit command', + kind: 'built-in', + action: quitAction, + }; + const result = setupProcessorHook([command]); + + await waitFor(() => + expect(result.current.slashCommands).toHaveLength(1), + ); + + vi.useFakeTimers(); + + try { + await act(async () => { + await result.current.handleSlashCommand('/exit'); + }); + + await act(async () => { + await vi.advanceTimersByTimeAsync(200); + }); + + expect(mockSetQuittingMessages).toHaveBeenCalledWith([]); + expect(mockProcessExit).toHaveBeenCalledWith(0); + } finally { + vi.useRealTimers(); + } + }); + }); + }); + + describe('Command Parsing and Matching', () => { + it('should be case-sensitive', async () => { + const command: SlashCommand = { + name: 'test', + description: 'a test command', + kind: 'built-in', + }; + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + // Use uppercase when command is lowercase + await result.current.handleSlashCommand('/Test'); + }); + + // It should fail and call addItem with an error + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.ERROR, + text: 'Unknown command: /Test', + }), + expect.any(Number), + ); + }); + + it('should correctly match an altName', async () => { + const action = vi.fn(); + const command: SlashCommand = { + name: 'main', + altNames: ['alias'], + description: 'a command with an alias', + kind: 'built-in', + action, + }; + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + await result.current.handleSlashCommand('/alias'); + }); + + expect(action).toHaveBeenCalledTimes(1); + expect(mockAddItem).not.toHaveBeenCalledWith( + expect.objectContaining({ type: MessageType.ERROR }), + ); + }); + + it('should handle extra whitespace around the command', async () => { + const action = vi.fn(); + const command: SlashCommand = { + name: 'test', + description: 'a test command', + kind: 'built-in', + action, + }; + const result = setupProcessorHook([command]); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + + await act(async () => { + await result.current.handleSlashCommand(' /test with-args '); + }); + + expect(action).toHaveBeenCalledWith(expect.anything(), 'with-args'); + }); + }); + + describe('Lifecycle', () => { + it('should abort command loading when the hook unmounts', async () => { + const abortSpy = vi.spyOn(AbortController.prototype, 'abort'); + const { unmount } = renderHook(() => + useSlashCommandProcessor( + mockConfig, + mockSettings, + mockAddItem, + mockClearItems, + mockLoadHistory, + vi.fn(), // refreshStatic + mockSetShowHelp, + vi.fn(), // onDebugMessage + vi.fn(), // openThemeDialog + mockOpenAuthDialog, + vi.fn(), // openEditorDialog + vi.fn(), // toggleCorgiMode + mockSetQuittingMessages, + vi.fn(), // openPrivacyNotice + ), + ); + + unmount(); + + expect(abortSpy).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index b56adeaf..cdf071b1 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -21,6 +21,7 @@ import { import { LoadedSettings } from '../../config/settings.js'; import { type CommandContext, type SlashCommand } from '../commands/types.js'; import { CommandService } from '../../services/CommandService.js'; +import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js'; /** * Hook to define and process slash commands (e.g., /help, /clear). @@ -42,7 +43,7 @@ export const useSlashCommandProcessor = ( openPrivacyNotice: () => void, ) => { const session = useSessionStats(); - const [commands, setCommands] = useState([]); + const [commands, setCommands] = useState([]); const gitService = useMemo(() => { if (!config?.getProjectRoot()) { return; @@ -158,16 +159,24 @@ export const useSlashCommandProcessor = ( ], ); - const commandService = useMemo(() => new CommandService(config), [config]); - useEffect(() => { + const controller = new AbortController(); const load = async () => { - await commandService.loadCommands(); + // TODO - Add other loaders for custom commands. + const loaders = [new BuiltinCommandLoader(config)]; + const commandService = await CommandService.create( + loaders, + controller.signal, + ); setCommands(commandService.getCommands()); }; load(); - }, [commandService]); + + return () => { + controller.abort(); + }; + }, [config]); const handleSlashCommand = useCallback( async ( @@ -199,7 +208,7 @@ export const useSlashCommandProcessor = ( for (const part of commandPath) { const foundCommand = currentCommands.find( - (cmd) => cmd.name === part || cmd.altName === part, + (cmd) => cmd.name === part || cmd.altNames?.includes(part), ); if (foundCommand) { diff --git a/packages/cli/src/ui/hooks/useCompletion.integration.test.ts b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts index f6f0944b..02162159 100644 --- a/packages/cli/src/ui/hooks/useCompletion.integration.test.ts +++ b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts @@ -53,13 +53,13 @@ describe('useCompletion git-aware filtering integration', () => { const mockSlashCommands: SlashCommand[] = [ { name: 'help', - altName: '?', + altNames: ['?'], description: 'Show help', action: vi.fn(), }, { name: 'stats', - altName: 'usage', + altNames: ['usage'], description: 'check session stats. Usage: /stats [model|tools]', action: vi.fn(), }, @@ -553,7 +553,7 @@ describe('useCompletion git-aware filtering integration', () => { }); it.each([['/?'], ['/usage']])( - 'should not suggest commands when altName is fully typed', + 'should not suggest commands when altNames is fully typed', async (altName) => { const { result } = renderHook(() => useCompletion( @@ -569,7 +569,7 @@ describe('useCompletion git-aware filtering integration', () => { }, ); - it('should suggest commands based on partial altName matches', async () => { + it('should suggest commands based on partial altNames matches', async () => { const { result } = renderHook(() => useCompletion( '/usag', // part of the word "usage" diff --git a/packages/cli/src/ui/hooks/useCompletion.test.ts b/packages/cli/src/ui/hooks/useCompletion.test.ts index f4227c1a..d1b22a88 100644 --- a/packages/cli/src/ui/hooks/useCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useCompletion.test.ts @@ -66,13 +66,13 @@ describe('useCompletion', () => { mockSlashCommands = [ { name: 'help', - altName: '?', + altNames: ['?'], description: 'Show help', action: vi.fn(), }, { name: 'stats', - altName: 'usage', + altNames: ['usage'], description: 'check session stats. Usage: /stats [model|tools]', action: vi.fn(), }, @@ -410,7 +410,7 @@ describe('useCompletion', () => { }); it.each([['/?'], ['/usage']])( - 'should not suggest commands when altName is fully typed', + 'should not suggest commands when altNames is fully typed', (altName) => { const { result } = renderHook(() => useCompletion( @@ -427,7 +427,7 @@ describe('useCompletion', () => { }, ); - it('should suggest commands based on partial altName matches', () => { + it('should suggest commands based on partial altNames matches', () => { const { result } = renderHook(() => useCompletion( '/usag', // part of the word "usage" diff --git a/packages/cli/src/ui/hooks/useCompletion.ts b/packages/cli/src/ui/hooks/useCompletion.ts index 69d8bfb9..aacc111d 100644 --- a/packages/cli/src/ui/hooks/useCompletion.ts +++ b/packages/cli/src/ui/hooks/useCompletion.ts @@ -41,7 +41,7 @@ export function useCompletion( query: string, cwd: string, isActive: boolean, - slashCommands: SlashCommand[], + slashCommands: readonly SlashCommand[], commandContext: CommandContext, config?: Config, ): UseCompletionReturn { @@ -151,7 +151,7 @@ export function useCompletion( } // Traverse the Command Tree using the tentative completed path - let currentLevel: SlashCommand[] | undefined = slashCommands; + let currentLevel: readonly SlashCommand[] | undefined = slashCommands; let leafCommand: SlashCommand | null = null; for (const part of commandPathParts) { @@ -161,11 +161,13 @@ export function useCompletion( break; } const found: SlashCommand | undefined = currentLevel.find( - (cmd) => cmd.name === part || cmd.altName === part, + (cmd) => cmd.name === part || cmd.altNames?.includes(part), ); if (found) { leafCommand = found; - currentLevel = found.subCommands; + currentLevel = found.subCommands as + | readonly SlashCommand[] + | undefined; } else { leafCommand = null; currentLevel = []; @@ -177,7 +179,7 @@ export function useCompletion( if (!hasTrailingSpace && currentLevel) { const exactMatchAsParent = currentLevel.find( (cmd) => - (cmd.name === partial || cmd.altName === partial) && + (cmd.name === partial || cmd.altNames?.includes(partial)) && cmd.subCommands, ); @@ -199,7 +201,8 @@ export function useCompletion( // Case: /command subcommand const perfectMatch = currentLevel.find( (cmd) => - (cmd.name === partial || cmd.altName === partial) && cmd.action, + (cmd.name === partial || cmd.altNames?.includes(partial)) && + cmd.action, ); if (perfectMatch) { setIsPerfectMatch(true); @@ -238,14 +241,15 @@ export function useCompletion( let potentialSuggestions = commandsToSearch.filter( (cmd) => cmd.description && - (cmd.name.startsWith(partial) || cmd.altName?.startsWith(partial)), + (cmd.name.startsWith(partial) || + cmd.altNames?.some((alt) => alt.startsWith(partial))), ); // If a user's input is an exact match and it is a leaf command, // enter should submit immediately. if (potentialSuggestions.length > 0 && !hasTrailingSpace) { const perfectMatch = potentialSuggestions.find( - (s) => s.name === partial || s.altName === partial, + (s) => s.name === partial || s.altNames?.includes(partial), ); if (perfectMatch && perfectMatch.action) { potentialSuggestions = [];