diff options
| author | Jacob Richman <[email protected]> | 2025-08-13 11:13:18 -0700 |
|---|---|---|
| committer | GitHub <[email protected]> | 2025-08-13 18:13:18 +0000 |
| commit | e4473a9007e7555809ec9e4087be636e8aadf017 (patch) | |
| tree | 9273204712b0eaf922589a473128aa886e0c207b /packages/cli/src | |
| parent | 38876b738f4c9ef8bd1b839d5e33580486e9a089 (diff) | |
Revert "chore(cli/slashcommands): Add status enum to SlashCommandEvent telemetry" (#6161)
Diffstat (limited to 'packages/cli/src')
| -rw-r--r-- | packages/cli/src/ui/hooks/slashCommandProcessor.test.ts | 110 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/slashCommandProcessor.ts | 124 |
2 files changed, 87 insertions, 147 deletions
diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 24880fc1..66c1b883 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -4,17 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -const { logSlashCommand } = vi.hoisted(() => ({ +const { logSlashCommand, SlashCommandEvent } = vi.hoisted(() => ({ logSlashCommand: vi.fn(), + SlashCommandEvent: vi.fn((command, subCommand) => ({ command, subCommand })), })); vi.mock('@google/gemini-cli-core', async (importOriginal) => { const original = await importOriginal<typeof import('@google/gemini-cli-core')>(); - return { ...original, logSlashCommand, + SlashCommandEvent, getIdeInstaller: vi.fn().mockReturnValue(null), }; }); @@ -24,10 +25,10 @@ const { mockProcessExit } = vi.hoisted(() => ({ })); vi.mock('node:process', () => { - const mockProcess: Partial<NodeJS.Process> = { + const mockProcess = { exit: mockProcessExit, - platform: 'sunos', - } as unknown as NodeJS.Process; + platform: 'test-platform', + }; return { ...mockProcess, default: mockProcess, @@ -76,28 +77,22 @@ import { ConfirmShellCommandsActionReturn, SlashCommand, } from '../commands/types.js'; -import { ToolConfirmationOutcome } from '@google/gemini-cli-core'; +import { Config, ToolConfirmationOutcome } from '@google/gemini-cli-core'; import { LoadedSettings } from '../../config/settings.js'; import { MessageType } from '../types.js'; import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js'; import { FileCommandLoader } from '../../services/FileCommandLoader.js'; import { McpPromptLoader } from '../../services/McpPromptLoader.js'; -import { - SlashCommandStatus, - makeFakeConfig, -} from '@google/gemini-cli-core/index.js'; -function createTestCommand( +const createTestCommand = ( overrides: Partial<SlashCommand>, kind: CommandKind = CommandKind.BUILT_IN, -): SlashCommand { - return { - name: 'test', - description: 'a test command', - kind, - ...overrides, - }; -} +): SlashCommand => ({ + name: 'test', + description: 'a test command', + kind, + ...overrides, +}); describe('useSlashCommandProcessor', () => { const mockAddItem = vi.fn(); @@ -107,7 +102,15 @@ describe('useSlashCommandProcessor', () => { const mockOpenAuthDialog = vi.fn(); const mockSetQuittingMessages = vi.fn(); - const mockConfig = makeFakeConfig({}); + const mockConfig = { + getProjectRoot: vi.fn(() => '/mock/cwd'), + getSessionId: vi.fn(() => 'test-session'), + getGeminiClient: vi.fn(() => ({ + setHistory: vi.fn().mockResolvedValue(undefined), + })), + getExtensions: vi.fn(() => []), + getIdeMode: vi.fn(() => false), + } as unknown as Config; const mockSettings = {} as LoadedSettings; @@ -881,9 +884,7 @@ describe('useSlashCommandProcessor', () => { const loggingTestCommands: SlashCommand[] = [ createTestCommand({ name: 'logtest', - action: vi - .fn() - .mockResolvedValue({ type: 'message', content: 'hello world' }), + action: mockCommandAction, }), createTestCommand({ name: 'logwithsub', @@ -895,10 +896,6 @@ describe('useSlashCommandProcessor', () => { ], }), createTestCommand({ - name: 'fail', - action: vi.fn().mockRejectedValue(new Error('oh no!')), - }), - createTestCommand({ name: 'logalias', altNames: ['la'], action: mockCommandAction, @@ -908,6 +905,7 @@ describe('useSlashCommandProcessor', () => { beforeEach(() => { mockCommandAction.mockClear(); vi.mocked(logSlashCommand).mockClear(); + vi.mocked(SlashCommandEvent).mockClear(); }); it('should log a simple slash command', async () => { @@ -919,45 +917,8 @@ describe('useSlashCommandProcessor', () => { await result.current.handleSlashCommand('/logtest'); }); - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ - command: 'logtest', - subcommand: undefined, - status: SlashCommandStatus.SUCCESS, - }), - ); - }); - - it('logs nothing for a bogus command', async () => { - const result = setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/bogusbogusbogus'); - }); - - expect(logSlashCommand).not.toHaveBeenCalled(); - }); - - it('logs a failure event for a failed command', async () => { - const result = setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/fail'); - }); - - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ - command: 'fail', - status: 'error', - subcommand: undefined, - }), - ); + expect(logSlashCommand).toHaveBeenCalledTimes(1); + expect(SlashCommandEvent).toHaveBeenCalledWith('logtest', undefined); }); it('should log a slash command with a subcommand', async () => { @@ -969,13 +930,8 @@ describe('useSlashCommandProcessor', () => { await result.current.handleSlashCommand('/logwithsub sub'); }); - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ - command: 'logwithsub', - subcommand: 'sub', - }), - ); + expect(logSlashCommand).toHaveBeenCalledTimes(1); + expect(SlashCommandEvent).toHaveBeenCalledWith('logwithsub', 'sub'); }); it('should log the command path when an alias is used', async () => { @@ -986,12 +942,8 @@ describe('useSlashCommandProcessor', () => { await act(async () => { await result.current.handleSlashCommand('/la'); }); - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ - command: 'logalias', - }), - ); + expect(logSlashCommand).toHaveBeenCalledTimes(1); + expect(SlashCommandEvent).toHaveBeenCalledWith('logalias', undefined); }); it('should not log for unknown commands', async () => { diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index aaa2fbff..b4ce0d4d 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -14,8 +14,7 @@ import { GitService, Logger, logSlashCommand, - makeSlashCommandEvent, - SlashCommandStatus, + SlashCommandEvent, ToolConfirmationOutcome, } from '@google/gemini-cli-core'; import { useSessionStats } from '../contexts/SessionContext.js'; @@ -230,70 +229,76 @@ export const useSlashCommandProcessor = ( overwriteConfirmed?: boolean, ): Promise<SlashCommandProcessorResult | false> => { setIsProcessing(true); + try { + if (typeof rawQuery !== 'string') { + return false; + } - if (typeof rawQuery !== 'string') { - return false; - } - - const trimmed = rawQuery.trim(); - if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) { - return false; - } + const trimmed = rawQuery.trim(); + if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) { + return false; + } - const userMessageTimestamp = Date.now(); - addItem({ type: MessageType.USER, text: trimmed }, userMessageTimestamp); + const userMessageTimestamp = Date.now(); + addItem( + { type: MessageType.USER, text: trimmed }, + userMessageTimestamp, + ); - const parts = trimmed.substring(1).trim().split(/\s+/); - const commandPath = parts.filter((p) => p); // The parts of the command, e.g., ['memory', 'add'] + const parts = trimmed.substring(1).trim().split(/\s+/); + const commandPath = parts.filter((p) => p); // The parts of the command, e.g., ['memory', 'add'] - let currentCommands = commands; - let commandToExecute: SlashCommand | undefined; - let pathIndex = 0; - let hasError = false; - const canonicalPath: string[] = []; + let currentCommands = commands; + let commandToExecute: SlashCommand | undefined; + let pathIndex = 0; + const canonicalPath: string[] = []; - for (const part of commandPath) { - // TODO: For better performance and architectural clarity, this two-pass - // search could be replaced. A more optimal approach would be to - // pre-compute a single lookup map in `CommandService.ts` that resolves - // all name and alias conflicts during the initial loading phase. The - // processor would then perform a single, fast lookup on that map. + for (const part of commandPath) { + // TODO: For better performance and architectural clarity, this two-pass + // search could be replaced. A more optimal approach would be to + // pre-compute a single lookup map in `CommandService.ts` that resolves + // all name and alias conflicts during the initial loading phase. The + // processor would then perform a single, fast lookup on that map. - // First pass: check for an exact match on the primary command name. - let foundCommand = currentCommands.find((cmd) => cmd.name === part); + // First pass: check for an exact match on the primary command name. + let foundCommand = currentCommands.find((cmd) => cmd.name === part); - // Second pass: if no primary name matches, check for an alias. - if (!foundCommand) { - foundCommand = currentCommands.find((cmd) => - cmd.altNames?.includes(part), - ); - } + // Second pass: if no primary name matches, check for an alias. + if (!foundCommand) { + foundCommand = currentCommands.find((cmd) => + cmd.altNames?.includes(part), + ); + } - if (foundCommand) { - commandToExecute = foundCommand; - canonicalPath.push(foundCommand.name); - pathIndex++; - if (foundCommand.subCommands) { - currentCommands = foundCommand.subCommands; + if (foundCommand) { + commandToExecute = foundCommand; + canonicalPath.push(foundCommand.name); + pathIndex++; + if (foundCommand.subCommands) { + currentCommands = foundCommand.subCommands; + } else { + break; + } } else { break; } - } else { - break; } - } - - const resolvedCommandPath = canonicalPath; - const subcommand = - resolvedCommandPath.length > 1 - ? resolvedCommandPath.slice(1).join(' ') - : undefined; - try { if (commandToExecute) { const args = parts.slice(pathIndex).join(' '); if (commandToExecute.action) { + if (config) { + const resolvedCommandPath = canonicalPath; + const event = new SlashCommandEvent( + resolvedCommandPath[0], + resolvedCommandPath.length > 1 + ? resolvedCommandPath.slice(1).join(' ') + : undefined, + ); + logSlashCommand(config, event); + } + const fullCommandContext: CommandContext = { ...commandContext, invocation: { @@ -315,6 +320,7 @@ export const useSlashCommandProcessor = ( ]), }; } + const result = await commandToExecute.action( fullCommandContext, args, @@ -487,18 +493,8 @@ export const useSlashCommandProcessor = ( content: `Unknown command: ${trimmed}`, timestamp: new Date(), }); - return { type: 'handled' }; - } catch (e: unknown) { - hasError = true; - if (config) { - const event = makeSlashCommandEvent({ - command: resolvedCommandPath[0], - subcommand, - status: SlashCommandStatus.ERROR, - }); - logSlashCommand(config, event); - } + } catch (e) { addItem( { type: MessageType.ERROR, @@ -508,14 +504,6 @@ export const useSlashCommandProcessor = ( ); return { type: 'handled' }; } finally { - if (config && resolvedCommandPath[0] && !hasError) { - const event = makeSlashCommandEvent({ - command: resolvedCommandPath[0], - subcommand, - status: SlashCommandStatus.SUCCESS, - }); - logSlashCommand(config, event); - } setIsProcessing(false); } }, |
