From ca07b5b0c41defb27d6b156faba356ab900ee092 Mon Sep 17 00:00:00 2001 From: Abhi <43648792+abhipatel12@users.noreply.github.com> Date: Thu, 17 Jul 2025 19:40:36 -0400 Subject: [PATCH] Migrate /corgi (#4419) --- .../cli/src/services/CommandService.test.ts | 10 +- packages/cli/src/services/CommandService.ts | 2 + .../cli/src/test-utils/mockCommandContext.ts | 1 + packages/cli/src/ui/App.tsx | 1 - .../cli/src/ui/commands/corgiCommand.test.ts | 34 +++++ packages/cli/src/ui/commands/corgiCommand.ts | 15 +++ packages/cli/src/ui/commands/types.ts | 3 + .../ui/hooks/slashCommandProcessor.test.ts | 32 ++--- .../cli/src/ui/hooks/slashCommandProcessor.ts | 116 +----------------- 9 files changed, 76 insertions(+), 138 deletions(-) create mode 100644 packages/cli/src/ui/commands/corgiCommand.test.ts create mode 100644 packages/cli/src/ui/commands/corgiCommand.ts diff --git a/packages/cli/src/services/CommandService.test.ts b/packages/cli/src/services/CommandService.test.ts index 6ae52b52..d03bf988 100644 --- a/packages/cli/src/services/CommandService.test.ts +++ b/packages/cli/src/services/CommandService.test.ts @@ -11,6 +11,7 @@ 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 { 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'; @@ -38,6 +39,9 @@ vi.mock('../ui/commands/helpCommand.js', () => ({ 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' }, })); @@ -85,7 +89,7 @@ vi.mock('../ui/commands/restoreCommand.js', () => ({ })); describe('CommandService', () => { - const subCommandLen = 17; + const subCommandLen = 18; let mockConfig: Mocked; beforeEach(() => { @@ -128,6 +132,8 @@ describe('CommandService', () => { expect(commandNames).toContain('memory'); expect(commandNames).toContain('help'); expect(commandNames).toContain('clear'); + expect(commandNames).toContain('compress'); + expect(commandNames).toContain('corgi'); expect(commandNames).toContain('docs'); expect(commandNames).toContain('chat'); expect(commandNames).toContain('theme'); @@ -136,7 +142,6 @@ describe('CommandService', () => { expect(commandNames).toContain('about'); expect(commandNames).toContain('extensions'); expect(commandNames).toContain('tools'); - expect(commandNames).toContain('compress'); expect(commandNames).toContain('mcp'); expect(commandNames).not.toContain('ide'); }); @@ -201,6 +206,7 @@ describe('CommandService', () => { chatCommand, clearCommand, compressCommand, + corgiCommand, docsCommand, editorCommand, extensionsCommand, diff --git a/packages/cli/src/services/CommandService.ts b/packages/cli/src/services/CommandService.ts index 611b0a7b..def8cfcc 100644 --- a/packages/cli/src/services/CommandService.ts +++ b/packages/cli/src/services/CommandService.ts @@ -9,6 +9,7 @@ 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 { 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'; @@ -36,6 +37,7 @@ const loadBuiltInCommands = async ( chatCommand, clearCommand, compressCommand, + corgiCommand, docsCommand, editorCommand, extensionsCommand, diff --git a/packages/cli/src/test-utils/mockCommandContext.ts b/packages/cli/src/test-utils/mockCommandContext.ts index 3fb33b3f..b760122b 100644 --- a/packages/cli/src/test-utils/mockCommandContext.ts +++ b/packages/cli/src/test-utils/mockCommandContext.ts @@ -47,6 +47,7 @@ export const createMockCommandContext = ( pendingItem: null, setPendingItem: vi.fn(), loadHistory: vi.fn(), + toggleCorgiMode: vi.fn(), }, session: { stats: { diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index e2856199..713ada01 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -379,7 +379,6 @@ const App = ({ config, settings, startupWarnings = [], version }: AppProps) => { } = useSlashCommandProcessor( config, settings, - history, addItem, clearItems, loadHistory, diff --git a/packages/cli/src/ui/commands/corgiCommand.test.ts b/packages/cli/src/ui/commands/corgiCommand.test.ts new file mode 100644 index 00000000..3c25e8cd --- /dev/null +++ b/packages/cli/src/ui/commands/corgiCommand.test.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { corgiCommand } from './corgiCommand.js'; +import { type CommandContext } from './types.js'; +import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; + +describe('corgiCommand', () => { + let mockContext: CommandContext; + + beforeEach(() => { + mockContext = createMockCommandContext(); + vi.spyOn(mockContext.ui, 'toggleCorgiMode'); + }); + + it('should call the toggleCorgiMode function on the UI context', async () => { + if (!corgiCommand.action) { + throw new Error('The corgi command must have an action.'); + } + + await corgiCommand.action(mockContext, ''); + + expect(mockContext.ui.toggleCorgiMode).toHaveBeenCalledTimes(1); + }); + + it('should have the correct name and description', () => { + expect(corgiCommand.name).toBe('corgi'); + expect(corgiCommand.description).toBe('Toggles corgi mode.'); + }); +}); diff --git a/packages/cli/src/ui/commands/corgiCommand.ts b/packages/cli/src/ui/commands/corgiCommand.ts new file mode 100644 index 00000000..290c071e --- /dev/null +++ b/packages/cli/src/ui/commands/corgiCommand.ts @@ -0,0 +1,15 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { type SlashCommand } from './types.js'; + +export const corgiCommand: SlashCommand = { + name: 'corgi', + description: 'Toggles corgi mode.', + action: (context, _args) => { + context.ui.toggleCorgiMode(); + }, +}; diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index 51b66fb4..3e269cbf 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -47,6 +47,8 @@ export interface CommandContext { * @param history The array of history items to load. */ loadHistory: UseHistoryManagerReturn['loadHistory']; + /** Toggles a special display mode. */ + toggleCorgiMode: () => void; }; // Session-specific data session: { @@ -103,6 +105,7 @@ export type SlashCommandActionReturn = | QuitActionReturn | OpenDialogActionReturn | LoadHistoryActionReturn; + // The standardized contract for any command in the system. export interface SlashCommand { name: string; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 38d17fc9..4920b088 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -14,7 +14,7 @@ vi.mock('node:process', () => ({ cwd: vi.fn(() => '/mock/cwd'), get env() { return process.env; - }, // Use a getter to ensure current process.env is used + }, platform: 'test-platform', version: 'test-node-version', memoryUsage: vi.fn(() => ({ @@ -25,12 +25,11 @@ vi.mock('node:process', () => ({ arrayBuffers: 123456, })), }, - // Provide top-level exports as well for compatibility exit: mockProcessExit, cwd: vi.fn(() => '/mock/cwd'), get env() { return process.env; - }, // Use a getter here too + }, platform: 'test-platform', version: 'test-node-version', memoryUsage: vi.fn(() => ({ @@ -106,20 +105,8 @@ describe('useSlashCommandProcessor', () => { const mockUseSessionStats = useSessionStats as Mock; beforeEach(() => { - // Reset all mocks to clear any previous state or calls. vi.clearAllMocks(); - // Default mock setup for CommandService for all the OLD tests. - // This makes them pass again by simulating the original behavior where - // the service is constructed but doesn't do much yet. - vi.mocked(CommandService).mockImplementation( - () => - ({ - loadCommands: vi.fn().mockResolvedValue(undefined), - getCommands: vi.fn().mockReturnValue([]), // Return an empty array by default - }) as unknown as CommandService, - ); - mockAddItem = vi.fn(); mockClearItems = vi.fn(); mockLoadHistory = vi.fn(); @@ -177,7 +164,6 @@ describe('useSlashCommandProcessor', () => { useSlashCommandProcessor( mockConfig, settings, - [], mockAddItem, mockClearItems, mockLoadHistory, @@ -194,7 +180,7 @@ describe('useSlashCommandProcessor', () => { ); }; - describe('New command registry', () => { + describe('Command Processing', () => { let ActualCommandService: typeof CommandService; beforeAll(async () => { @@ -208,7 +194,7 @@ describe('useSlashCommandProcessor', () => { vi.clearAllMocks(); }); - it('should execute a command from the new registry', async () => { + it('should execute a registered command', async () => { const mockAction = vi.fn(); const newCommand: SlashCommand = { name: 'test', action: mockAction }; const mockLoader = async () => [newCommand]; @@ -243,7 +229,7 @@ describe('useSlashCommandProcessor', () => { expect(commandResult).toEqual({ type: 'handled' }); }); - it('should return "schedule_tool" when a new command returns a tool action', async () => { + it('should return "schedule_tool" for a command returning a tool action', async () => { const mockAction = vi.fn().mockResolvedValue({ type: 'tool', toolName: 'my_tool', @@ -276,7 +262,7 @@ describe('useSlashCommandProcessor', () => { }); }); - it('should return "handled" when a new command returns a message action', async () => { + it('should return "handled" for a command returning a message action', async () => { const mockAction = vi.fn().mockResolvedValue({ type: 'message', messageType: 'info', @@ -312,7 +298,7 @@ describe('useSlashCommandProcessor', () => { expect(commandResult).toEqual({ type: 'handled' }); }); - it('should return "handled" when a new command returns a dialog action', async () => { + it('should return "handled" for a command returning a dialog action', async () => { const mockAction = vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'help', @@ -341,7 +327,7 @@ describe('useSlashCommandProcessor', () => { expect(commandResult).toEqual({ type: 'handled' }); }); - it('should open the auth dialog when a new command returns an auth dialog action', async () => { + it('should open the auth dialog for a command returning an auth dialog action', async () => { const mockAction = vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'auth', @@ -371,7 +357,7 @@ describe('useSlashCommandProcessor', () => { expect(commandResult).toEqual({ type: 'handled' }); }); - it('should open the theme dialog when a new command returns a theme dialog action', async () => { + it('should open the theme dialog for a command returning a theme dialog action', async () => { const mockAction = vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'theme', diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 295d1c50..b56adeaf 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -19,37 +19,15 @@ import { SlashCommandProcessorResult, } from '../types.js'; import { LoadedSettings } from '../../config/settings.js'; -import { - type CommandContext, - type SlashCommandActionReturn, - type SlashCommand, -} from '../commands/types.js'; +import { type CommandContext, type SlashCommand } from '../commands/types.js'; import { CommandService } from '../../services/CommandService.js'; -// This interface is for the old, inline command definitions. -// It will be removed once all commands are migrated to the new system. -export interface LegacySlashCommand { - name: string; - altName?: string; - description?: string; - completion?: () => Promise; - action: ( - mainCommand: string, - subCommand?: string, - args?: string, - ) => - | void - | SlashCommandActionReturn - | Promise; -} - /** * Hook to define and process slash commands (e.g., /help, /clear). */ export const useSlashCommandProcessor = ( config: Config | null, settings: LoadedSettings, - history: HistoryItem[], addItem: UseHistoryManagerReturn['addItem'], clearItems: UseHistoryManagerReturn['clearItems'], loadHistory: UseHistoryManagerReturn['loadHistory'], @@ -157,6 +135,7 @@ export const useSlashCommandProcessor = ( setDebugMessage: onDebugMessage, pendingItem: pendingCompressionItemRef.current, setPendingItem: setPendingCompressionItem, + toggleCorgiMode, }, session: { stats: session.stats, @@ -175,6 +154,7 @@ export const useSlashCommandProcessor = ( onDebugMessage, pendingCompressionItemRef, setPendingCompressionItem, + toggleCorgiMode, ], ); @@ -189,23 +169,6 @@ export const useSlashCommandProcessor = ( load(); }, [commandService]); - // Define legacy commands - // This list contains all commands that have NOT YET been migrated to the - // new system. As commands are migrated, they are removed from this list. - const legacyCommands: LegacySlashCommand[] = useMemo(() => { - const commands: LegacySlashCommand[] = [ - // `/help` and `/clear` have been migrated and REMOVED from this list. - { - name: 'corgi', - action: (_mainCommand, _subCommand, _args) => { - toggleCorgiMode(); - }, - }, - ]; - - return commands; - }, [toggleCorgiMode]); - const handleSlashCommand = useCallback( async ( rawQuery: PartListUnion, @@ -230,8 +193,6 @@ export const useSlashCommandProcessor = ( const parts = trimmed.substring(1).trim().split(/\s+/); const commandPath = parts.filter((p) => p); // The parts of the command, e.g., ['memory', 'add'] - // --- Start of New Tree Traversal Logic --- - let currentCommands = commands; let commandToExecute: SlashCommand | undefined; let pathIndex = 0; @@ -341,45 +302,6 @@ export const useSlashCommandProcessor = ( } } - // --- End of New Tree Traversal Logic --- - - // --- Legacy Fallback Logic (for commands not yet migrated) --- - - const mainCommand = parts[0]; - const subCommand = parts[1]; - const legacyArgs = parts.slice(2).join(' '); - - for (const cmd of legacyCommands) { - if (mainCommand === cmd.name || mainCommand === cmd.altName) { - const actionResult = await cmd.action( - mainCommand, - subCommand, - legacyArgs, - ); - - if (actionResult?.type === 'tool') { - return { - type: 'schedule_tool', - toolName: actionResult.toolName, - toolArgs: actionResult.toolArgs, - }; - } - if (actionResult?.type === 'message') { - addItem( - { - type: - actionResult.messageType === 'error' - ? MessageType.ERROR - : MessageType.INFO, - text: actionResult.content, - }, - Date.now(), - ); - } - return { type: 'handled' }; - } - } - addMessage({ type: MessageType.ERROR, content: `Unknown command: ${trimmed}`, @@ -393,7 +315,6 @@ export const useSlashCommandProcessor = ( setShowHelp, openAuthDialog, commands, - legacyCommands, commandContext, addMessage, openThemeDialog, @@ -403,38 +324,9 @@ export const useSlashCommandProcessor = ( ], ); - const allCommands = useMemo(() => { - // Adapt legacy commands to the new SlashCommand interface - const adaptedLegacyCommands: SlashCommand[] = legacyCommands.map( - (legacyCmd) => ({ - name: legacyCmd.name, - altName: legacyCmd.altName, - description: legacyCmd.description, - action: async (_context: CommandContext, args: string) => { - const parts = args.split(/\s+/); - const subCommand = parts[0] || undefined; - const restOfArgs = parts.slice(1).join(' ') || undefined; - - return legacyCmd.action(legacyCmd.name, subCommand, restOfArgs); - }, - completion: legacyCmd.completion - ? async (_context: CommandContext, _partialArg: string) => - legacyCmd.completion!() - : undefined, - }), - ); - - const newCommandNames = new Set(commands.map((c) => c.name)); - const filteredAdaptedLegacy = adaptedLegacyCommands.filter( - (c) => !newCommandNames.has(c.name), - ); - - return [...commands, ...filteredAdaptedLegacy]; - }, [commands, legacyCommands]); - return { handleSlashCommand, - slashCommands: allCommands, + slashCommands: commands, pendingHistoryItems, commandContext, };