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(); - return { ...original, logSlashCommand, + SlashCommandEvent, getIdeInstaller: vi.fn().mockReturnValue(null), }; }); @@ -24,10 +25,10 @@ const { mockProcessExit } = vi.hoisted(() => ({ })); vi.mock('node:process', () => { - const mockProcess: Partial = { + 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, 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', @@ -894,10 +895,6 @@ describe('useSlashCommandProcessor', () => { }), ], }), - createTestCommand({ - name: 'fail', - action: vi.fn().mockRejectedValue(new Error('oh no!')), - }), createTestCommand({ name: 'logalias', altNames: ['la'], @@ -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 => { setIsProcessing(true); - - 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), - ); + try { + if (typeof rawQuery !== 'string') { + return false; } - if (foundCommand) { - commandToExecute = foundCommand; - canonicalPath.push(foundCommand.name); - pathIndex++; - if (foundCommand.subCommands) { - currentCommands = foundCommand.subCommands; + 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; + } } 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); } }, diff --git a/packages/core/index.ts b/packages/core/index.ts index 7b75b365..65a214ae 100644 --- a/packages/core/index.ts +++ b/packages/core/index.ts @@ -15,4 +15,3 @@ 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 9450f06d..b7be2af7 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts @@ -639,13 +639,6 @@ 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 cb4172ed..314e61a8 100644 --- a/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts +++ b/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts @@ -174,9 +174,6 @@ 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 0f343ab3..33781b87 100644 --- a/packages/core/src/telemetry/index.ts +++ b/packages/core/src/telemetry/index.ts @@ -41,8 +41,6 @@ export { FlashFallbackEvent, SlashCommandEvent, KittySequenceOverflowEvent, - 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 b07c4ca4..2b10280d 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -14,17 +14,9 @@ import { ToolCallDecision, } from './tool-call-decision.js'; -interface BaseTelemetryEvent { - 'event.name': string; - /** Current timestamp in ISO 8601 format */ - 'event.timestamp': string; -} - -type CommonFields = keyof BaseTelemetryEvent; - -export class StartSessionEvent implements BaseTelemetryEvent { +export class StartSessionEvent { 'event.name': 'cli_config'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 model: string; embedding_model: string; sandbox_enabled: boolean; @@ -68,9 +60,9 @@ export class StartSessionEvent implements BaseTelemetryEvent { } } -export class EndSessionEvent implements BaseTelemetryEvent { +export class EndSessionEvent { 'event.name': 'end_session'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 session_id?: string; constructor(config?: Config) { @@ -80,9 +72,9 @@ export class EndSessionEvent implements BaseTelemetryEvent { } } -export class UserPromptEvent implements BaseTelemetryEvent { +export class UserPromptEvent { 'event.name': 'user_prompt'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 prompt_length: number; prompt_id: string; auth_type?: string; @@ -103,9 +95,9 @@ export class UserPromptEvent implements BaseTelemetryEvent { } } -export class ToolCallEvent implements BaseTelemetryEvent { +export class ToolCallEvent { 'event.name': 'tool_call'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 function_name: string; function_args: Record; duration_ms: number; @@ -150,9 +142,9 @@ export class ToolCallEvent implements BaseTelemetryEvent { } } -export class ApiRequestEvent implements BaseTelemetryEvent { +export class ApiRequestEvent { 'event.name': 'api_request'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 model: string; prompt_id: string; request_text?: string; @@ -166,9 +158,9 @@ export class ApiRequestEvent implements BaseTelemetryEvent { } } -export class ApiErrorEvent implements BaseTelemetryEvent { +export class ApiErrorEvent { 'event.name': 'api_error'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 model: string; error: string; error_type?: string; @@ -198,9 +190,9 @@ export class ApiErrorEvent implements BaseTelemetryEvent { } } -export class ApiResponseEvent implements BaseTelemetryEvent { +export class ApiResponseEvent { 'event.name': 'api_response'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 model: string; status_code?: number | string; duration_ms: number; @@ -242,9 +234,9 @@ export class ApiResponseEvent implements BaseTelemetryEvent { } } -export class FlashFallbackEvent implements BaseTelemetryEvent { +export class FlashFallbackEvent { 'event.name': 'flash_fallback'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 auth_type: string; constructor(auth_type: string) { @@ -260,9 +252,9 @@ export enum LoopType { LLM_DETECTED_LOOP = 'llm_detected_loop', } -export class LoopDetectedEvent implements BaseTelemetryEvent { +export class LoopDetectedEvent { 'event.name': 'loop_detected'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 loop_type: LoopType; prompt_id: string; @@ -274,9 +266,9 @@ export class LoopDetectedEvent implements BaseTelemetryEvent { } } -export class NextSpeakerCheckEvent implements BaseTelemetryEvent { +export class NextSpeakerCheckEvent { 'event.name': 'next_speaker_check'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 prompt_id: string; finish_reason: string; result: string; @@ -290,36 +282,23 @@ export class NextSpeakerCheckEvent implements BaseTelemetryEvent { } } -export interface SlashCommandEvent extends BaseTelemetryEvent { +export class SlashCommandEvent { 'event.name': 'slash_command'; 'event.timestamp': string; // ISO 8106 command: string; subcommand?: string; - status?: SlashCommandStatus; + + constructor(command: string, subcommand?: string) { + this['event.name'] = 'slash_command'; + this['event.timestamp'] = new Date().toISOString(); + this.command = command; + this.subcommand = subcommand; + } } -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 { +export class MalformedJsonResponseEvent { 'event.name': 'malformed_json_response'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 model: string; constructor(model: string) { @@ -336,7 +315,7 @@ export enum IdeConnectionType { export class IdeConnectionEvent { 'event.name': 'ide_connection'; - 'event.timestamp': string; + 'event.timestamp': string; // ISO 8601 connection_type: IdeConnectionType; constructor(connection_type: IdeConnectionType) {