diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index 66c1b883..24880fc1 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -4,18 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -const { logSlashCommand, SlashCommandEvent } = vi.hoisted(() => ({ +const { logSlashCommand } = vi.hoisted(() => ({ logSlashCommand: vi.fn(), - SlashCommandEvent: vi.fn((command, subCommand) => ({ command, subCommand })), })); vi.mock('@google/gemini-cli-core', async (importOriginal) => { const original = await importOriginal(); + return { ...original, logSlashCommand, - SlashCommandEvent, getIdeInstaller: vi.fn().mockReturnValue(null), }; }); @@ -25,10 +24,10 @@ const { mockProcessExit } = vi.hoisted(() => ({ })); vi.mock('node:process', () => { - const mockProcess = { + const mockProcess: Partial = { exit: mockProcessExit, - platform: 'test-platform', - }; + platform: 'sunos', + } as unknown as NodeJS.Process; return { ...mockProcess, default: mockProcess, @@ -77,22 +76,28 @@ import { ConfirmShellCommandsActionReturn, SlashCommand, } from '../commands/types.js'; -import { Config, ToolConfirmationOutcome } from '@google/gemini-cli-core'; +import { 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'; -const createTestCommand = ( +function createTestCommand( overrides: Partial, kind: CommandKind = CommandKind.BUILT_IN, -): SlashCommand => ({ - name: 'test', - description: 'a test command', - kind, - ...overrides, -}); +): SlashCommand { + return { + name: 'test', + description: 'a test command', + kind, + ...overrides, + }; +} describe('useSlashCommandProcessor', () => { const mockAddItem = vi.fn(); @@ -102,15 +107,7 @@ describe('useSlashCommandProcessor', () => { const mockOpenAuthDialog = vi.fn(); const mockSetQuittingMessages = vi.fn(); - 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 mockConfig = makeFakeConfig({}); const mockSettings = {} as LoadedSettings; @@ -884,7 +881,9 @@ describe('useSlashCommandProcessor', () => { const loggingTestCommands: SlashCommand[] = [ createTestCommand({ name: 'logtest', - action: mockCommandAction, + action: vi + .fn() + .mockResolvedValue({ type: 'message', content: 'hello world' }), }), createTestCommand({ name: 'logwithsub', @@ -895,6 +894,10 @@ describe('useSlashCommandProcessor', () => { }), ], }), + createTestCommand({ + name: 'fail', + action: vi.fn().mockRejectedValue(new Error('oh no!')), + }), createTestCommand({ name: 'logalias', altNames: ['la'], @@ -905,7 +908,6 @@ describe('useSlashCommandProcessor', () => { beforeEach(() => { mockCommandAction.mockClear(); vi.mocked(logSlashCommand).mockClear(); - vi.mocked(SlashCommandEvent).mockClear(); }); it('should log a simple slash command', async () => { @@ -917,8 +919,45 @@ describe('useSlashCommandProcessor', () => { await result.current.handleSlashCommand('/logtest'); }); - expect(logSlashCommand).toHaveBeenCalledTimes(1); - expect(SlashCommandEvent).toHaveBeenCalledWith('logtest', undefined); + 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, + }), + ); }); it('should log a slash command with a subcommand', async () => { @@ -930,8 +969,13 @@ describe('useSlashCommandProcessor', () => { await result.current.handleSlashCommand('/logwithsub sub'); }); - expect(logSlashCommand).toHaveBeenCalledTimes(1); - expect(SlashCommandEvent).toHaveBeenCalledWith('logwithsub', 'sub'); + expect(logSlashCommand).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + command: 'logwithsub', + subcommand: 'sub', + }), + ); }); it('should log the command path when an alias is used', async () => { @@ -942,8 +986,12 @@ describe('useSlashCommandProcessor', () => { await act(async () => { await result.current.handleSlashCommand('/la'); }); - expect(logSlashCommand).toHaveBeenCalledTimes(1); - expect(SlashCommandEvent).toHaveBeenCalledWith('logalias', undefined); + expect(logSlashCommand).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + command: 'logalias', + }), + ); }); 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 b4ce0d4d..aaa2fbff 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -14,7 +14,8 @@ import { GitService, Logger, logSlashCommand, - SlashCommandEvent, + makeSlashCommandEvent, + SlashCommandStatus, ToolConfirmationOutcome, } from '@google/gemini-cli-core'; import { useSessionStats } from '../contexts/SessionContext.js'; @@ -229,76 +230,70 @@ export const useSlashCommandProcessor = ( overwriteConfirmed?: boolean, ): Promise => { 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 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'] + + let currentCommands = commands; + let commandToExecute: SlashCommand | undefined; + let pathIndex = 0; + let hasError = false; + 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. + + // 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), + ); } - const trimmed = rawQuery.trim(); - if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) { - return false; - } - - 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'] - - 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. - - // 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), - ); - } - - if (foundCommand) { - commandToExecute = foundCommand; - canonicalPath.push(foundCommand.name); - pathIndex++; - if (foundCommand.subCommands) { - currentCommands = foundCommand.subCommands; - } else { - break; - } + if (foundCommand) { + commandToExecute = foundCommand; + canonicalPath.push(foundCommand.name); + pathIndex++; + if (foundCommand.subCommands) { + currentCommands = foundCommand.subCommands; } 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: { @@ -320,7 +315,6 @@ export const useSlashCommandProcessor = ( ]), }; } - const result = await commandToExecute.action( fullCommandContext, args, @@ -493,8 +487,18 @@ export const useSlashCommandProcessor = ( content: `Unknown command: ${trimmed}`, timestamp: new Date(), }); + return { type: 'handled' }; - } catch (e) { + } catch (e: unknown) { + hasError = true; + if (config) { + const event = makeSlashCommandEvent({ + command: resolvedCommandPath[0], + subcommand, + status: SlashCommandStatus.ERROR, + }); + logSlashCommand(config, event); + } addItem( { type: MessageType.ERROR, @@ -504,6 +508,14 @@ 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); } }, diff --git a/packages/core/index.ts b/packages/core/index.ts index 65a214ae..7b75b365 100644 --- a/packages/core/index.ts +++ b/packages/core/index.ts @@ -15,3 +15,4 @@ export { IdeConnectionEvent, IdeConnectionType, } from './src/telemetry/types.js'; +export { makeFakeConfig } from './src/test-utils/config.js'; diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts index a41f832d..0c13e864 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts @@ -637,6 +637,13 @@ export class ClearcutLogger { }); } + if (event.status) { + data.push({ + gemini_cli_key: EventMetadataKey.GEMINI_CLI_SLASH_COMMAND_STATUS, + value: JSON.stringify(event.status), + }); + } + this.enqueueLogEvent(this.createLogEvent(slash_command_event_name, data)); this.flushIfNeeded(); } diff --git a/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts b/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts index 81f41603..9dae3e0d 100644 --- a/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts +++ b/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts @@ -174,6 +174,9 @@ export enum EventMetadataKey { // Logs the subcommand of the slash command. GEMINI_CLI_SLASH_COMMAND_SUBCOMMAND = 42, + // Logs the status of the slash command (e.g. 'success', 'error') + GEMINI_CLI_SLASH_COMMAND_STATUS = 51, + // ========================================================================== // Next Speaker Check Event Keys // =========================================================================== diff --git a/packages/core/src/telemetry/index.ts b/packages/core/src/telemetry/index.ts index 6648b229..1663abdf 100644 --- a/packages/core/src/telemetry/index.ts +++ b/packages/core/src/telemetry/index.ts @@ -39,6 +39,8 @@ export { TelemetryEvent, FlashFallbackEvent, SlashCommandEvent, + makeSlashCommandEvent, + SlashCommandStatus, } from './types.js'; export { SpanStatusCode, ValueType } from '@opentelemetry/api'; export { SemanticAttributes } from '@opentelemetry/semantic-conventions'; diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index edcd8b1b..d590699c 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -14,9 +14,17 @@ import { ToolCallDecision, } from './tool-call-decision.js'; -export class StartSessionEvent { +interface BaseTelemetryEvent { + 'event.name': string; + /** Current timestamp in ISO 8601 format */ + 'event.timestamp': string; +} + +type CommonFields = keyof BaseTelemetryEvent; + +export class StartSessionEvent implements BaseTelemetryEvent { 'event.name': 'cli_config'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; model: string; embedding_model: string; sandbox_enabled: boolean; @@ -60,9 +68,9 @@ export class StartSessionEvent { } } -export class EndSessionEvent { +export class EndSessionEvent implements BaseTelemetryEvent { 'event.name': 'end_session'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; session_id?: string; constructor(config?: Config) { @@ -72,9 +80,9 @@ export class EndSessionEvent { } } -export class UserPromptEvent { +export class UserPromptEvent implements BaseTelemetryEvent { 'event.name': 'user_prompt'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; prompt_length: number; prompt_id: string; auth_type?: string; @@ -95,9 +103,9 @@ export class UserPromptEvent { } } -export class ToolCallEvent { +export class ToolCallEvent implements BaseTelemetryEvent { 'event.name': 'tool_call'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; function_name: string; function_args: Record; duration_ms: number; @@ -142,9 +150,9 @@ export class ToolCallEvent { } } -export class ApiRequestEvent { +export class ApiRequestEvent implements BaseTelemetryEvent { 'event.name': 'api_request'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; model: string; prompt_id: string; request_text?: string; @@ -158,9 +166,9 @@ export class ApiRequestEvent { } } -export class ApiErrorEvent { +export class ApiErrorEvent implements BaseTelemetryEvent { 'event.name': 'api_error'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; model: string; error: string; error_type?: string; @@ -190,9 +198,9 @@ export class ApiErrorEvent { } } -export class ApiResponseEvent { +export class ApiResponseEvent implements BaseTelemetryEvent { 'event.name': 'api_response'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; model: string; status_code?: number | string; duration_ms: number; @@ -234,9 +242,9 @@ export class ApiResponseEvent { } } -export class FlashFallbackEvent { +export class FlashFallbackEvent implements BaseTelemetryEvent { 'event.name': 'flash_fallback'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; auth_type: string; constructor(auth_type: string) { @@ -252,9 +260,9 @@ export enum LoopType { LLM_DETECTED_LOOP = 'llm_detected_loop', } -export class LoopDetectedEvent { +export class LoopDetectedEvent implements BaseTelemetryEvent { 'event.name': 'loop_detected'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; loop_type: LoopType; prompt_id: string; @@ -266,9 +274,9 @@ export class LoopDetectedEvent { } } -export class NextSpeakerCheckEvent { +export class NextSpeakerCheckEvent implements BaseTelemetryEvent { 'event.name': 'next_speaker_check'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; prompt_id: string; finish_reason: string; result: string; @@ -282,23 +290,36 @@ export class NextSpeakerCheckEvent { } } -export class SlashCommandEvent { +export interface SlashCommandEvent extends BaseTelemetryEvent { 'event.name': 'slash_command'; 'event.timestamp': string; // ISO 8106 command: string; subcommand?: string; - - constructor(command: string, subcommand?: string) { - this['event.name'] = 'slash_command'; - this['event.timestamp'] = new Date().toISOString(); - this.command = command; - this.subcommand = subcommand; - } + status?: SlashCommandStatus; } -export class MalformedJsonResponseEvent { +export function makeSlashCommandEvent({ + command, + subcommand, + status, +}: Omit): SlashCommandEvent { + return { + 'event.name': 'slash_command', + 'event.timestamp': new Date().toISOString(), + command, + subcommand, + status, + }; +} + +export enum SlashCommandStatus { + SUCCESS = 'success', + ERROR = 'error', +} + +export class MalformedJsonResponseEvent implements BaseTelemetryEvent { 'event.name': 'malformed_json_response'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; model: string; constructor(model: string) { @@ -315,7 +336,7 @@ export enum IdeConnectionType { export class IdeConnectionEvent { 'event.name': 'ide_connection'; - 'event.timestamp': string; // ISO 8601 + 'event.timestamp': string; connection_type: IdeConnectionType; constructor(connection_type: IdeConnectionType) { @@ -338,4 +359,5 @@ export type TelemetryEvent = | NextSpeakerCheckEvent | SlashCommandEvent | MalformedJsonResponseEvent - | IdeConnectionEvent; + | IdeConnectionEvent + | SlashCommandEvent;