Revert "chore(cli/slashcommands): Add status enum to SlashCommandEvent telemetry" (#6161)

This commit is contained in:
Jacob Richman 2025-08-13 11:13:18 -07:00 committed by GitHub
parent 38876b738f
commit e4473a9007
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 124 additions and 218 deletions

View File

@ -4,17 +4,18 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
const { logSlashCommand } = vi.hoisted(() => ({ const { logSlashCommand, SlashCommandEvent } = vi.hoisted(() => ({
logSlashCommand: vi.fn(), logSlashCommand: vi.fn(),
SlashCommandEvent: vi.fn((command, subCommand) => ({ command, subCommand })),
})); }));
vi.mock('@google/gemini-cli-core', async (importOriginal) => { vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const original = const original =
await importOriginal<typeof import('@google/gemini-cli-core')>(); await importOriginal<typeof import('@google/gemini-cli-core')>();
return { return {
...original, ...original,
logSlashCommand, logSlashCommand,
SlashCommandEvent,
getIdeInstaller: vi.fn().mockReturnValue(null), getIdeInstaller: vi.fn().mockReturnValue(null),
}; };
}); });
@ -24,10 +25,10 @@ const { mockProcessExit } = vi.hoisted(() => ({
})); }));
vi.mock('node:process', () => { vi.mock('node:process', () => {
const mockProcess: Partial<NodeJS.Process> = { const mockProcess = {
exit: mockProcessExit, exit: mockProcessExit,
platform: 'sunos', platform: 'test-platform',
} as unknown as NodeJS.Process; };
return { return {
...mockProcess, ...mockProcess,
default: mockProcess, default: mockProcess,
@ -76,28 +77,22 @@ import {
ConfirmShellCommandsActionReturn, ConfirmShellCommandsActionReturn,
SlashCommand, SlashCommand,
} from '../commands/types.js'; } 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 { LoadedSettings } from '../../config/settings.js';
import { MessageType } from '../types.js'; import { MessageType } from '../types.js';
import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js'; import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js';
import { FileCommandLoader } from '../../services/FileCommandLoader.js'; import { FileCommandLoader } from '../../services/FileCommandLoader.js';
import { McpPromptLoader } from '../../services/McpPromptLoader.js'; import { McpPromptLoader } from '../../services/McpPromptLoader.js';
import {
SlashCommandStatus,
makeFakeConfig,
} from '@google/gemini-cli-core/index.js';
function createTestCommand( const createTestCommand = (
overrides: Partial<SlashCommand>, overrides: Partial<SlashCommand>,
kind: CommandKind = CommandKind.BUILT_IN, kind: CommandKind = CommandKind.BUILT_IN,
): SlashCommand { ): SlashCommand => ({
return { name: 'test',
name: 'test', description: 'a test command',
description: 'a test command', kind,
kind, ...overrides,
...overrides, });
};
}
describe('useSlashCommandProcessor', () => { describe('useSlashCommandProcessor', () => {
const mockAddItem = vi.fn(); const mockAddItem = vi.fn();
@ -107,7 +102,15 @@ describe('useSlashCommandProcessor', () => {
const mockOpenAuthDialog = vi.fn(); const mockOpenAuthDialog = vi.fn();
const mockSetQuittingMessages = 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; const mockSettings = {} as LoadedSettings;
@ -881,9 +884,7 @@ describe('useSlashCommandProcessor', () => {
const loggingTestCommands: SlashCommand[] = [ const loggingTestCommands: SlashCommand[] = [
createTestCommand({ createTestCommand({
name: 'logtest', name: 'logtest',
action: vi action: mockCommandAction,
.fn()
.mockResolvedValue({ type: 'message', content: 'hello world' }),
}), }),
createTestCommand({ createTestCommand({
name: 'logwithsub', name: 'logwithsub',
@ -894,10 +895,6 @@ describe('useSlashCommandProcessor', () => {
}), }),
], ],
}), }),
createTestCommand({
name: 'fail',
action: vi.fn().mockRejectedValue(new Error('oh no!')),
}),
createTestCommand({ createTestCommand({
name: 'logalias', name: 'logalias',
altNames: ['la'], altNames: ['la'],
@ -908,6 +905,7 @@ describe('useSlashCommandProcessor', () => {
beforeEach(() => { beforeEach(() => {
mockCommandAction.mockClear(); mockCommandAction.mockClear();
vi.mocked(logSlashCommand).mockClear(); vi.mocked(logSlashCommand).mockClear();
vi.mocked(SlashCommandEvent).mockClear();
}); });
it('should log a simple slash command', async () => { it('should log a simple slash command', async () => {
@ -919,45 +917,8 @@ describe('useSlashCommandProcessor', () => {
await result.current.handleSlashCommand('/logtest'); await result.current.handleSlashCommand('/logtest');
}); });
expect(logSlashCommand).toHaveBeenCalledWith( expect(logSlashCommand).toHaveBeenCalledTimes(1);
mockConfig, expect(SlashCommandEvent).toHaveBeenCalledWith('logtest', undefined);
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 () => { it('should log a slash command with a subcommand', async () => {
@ -969,13 +930,8 @@ describe('useSlashCommandProcessor', () => {
await result.current.handleSlashCommand('/logwithsub sub'); await result.current.handleSlashCommand('/logwithsub sub');
}); });
expect(logSlashCommand).toHaveBeenCalledWith( expect(logSlashCommand).toHaveBeenCalledTimes(1);
mockConfig, expect(SlashCommandEvent).toHaveBeenCalledWith('logwithsub', 'sub');
expect.objectContaining({
command: 'logwithsub',
subcommand: 'sub',
}),
);
}); });
it('should log the command path when an alias is used', async () => { it('should log the command path when an alias is used', async () => {
@ -986,12 +942,8 @@ describe('useSlashCommandProcessor', () => {
await act(async () => { await act(async () => {
await result.current.handleSlashCommand('/la'); await result.current.handleSlashCommand('/la');
}); });
expect(logSlashCommand).toHaveBeenCalledWith( expect(logSlashCommand).toHaveBeenCalledTimes(1);
mockConfig, expect(SlashCommandEvent).toHaveBeenCalledWith('logalias', undefined);
expect.objectContaining({
command: 'logalias',
}),
);
}); });
it('should not log for unknown commands', async () => { it('should not log for unknown commands', async () => {

View File

@ -14,8 +14,7 @@ import {
GitService, GitService,
Logger, Logger,
logSlashCommand, logSlashCommand,
makeSlashCommandEvent, SlashCommandEvent,
SlashCommandStatus,
ToolConfirmationOutcome, ToolConfirmationOutcome,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import { useSessionStats } from '../contexts/SessionContext.js'; import { useSessionStats } from '../contexts/SessionContext.js';
@ -230,70 +229,76 @@ export const useSlashCommandProcessor = (
overwriteConfirmed?: boolean, overwriteConfirmed?: boolean,
): Promise<SlashCommandProcessorResult | false> => { ): Promise<SlashCommandProcessorResult | false> => {
setIsProcessing(true); setIsProcessing(true);
try {
if (typeof rawQuery !== 'string') { if (typeof rawQuery !== 'string') {
return false; 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),
);
} }
if (foundCommand) { const trimmed = rawQuery.trim();
commandToExecute = foundCommand; if (!trimmed.startsWith('/') && !trimmed.startsWith('?')) {
canonicalPath.push(foundCommand.name); return false;
pathIndex++; }
if (foundCommand.subCommands) {
currentCommands = foundCommand.subCommands; 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 { } else {
break; break;
} }
} else {
break;
} }
}
const resolvedCommandPath = canonicalPath;
const subcommand =
resolvedCommandPath.length > 1
? resolvedCommandPath.slice(1).join(' ')
: undefined;
try {
if (commandToExecute) { if (commandToExecute) {
const args = parts.slice(pathIndex).join(' '); const args = parts.slice(pathIndex).join(' ');
if (commandToExecute.action) { 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 = { const fullCommandContext: CommandContext = {
...commandContext, ...commandContext,
invocation: { invocation: {
@ -315,6 +320,7 @@ export const useSlashCommandProcessor = (
]), ]),
}; };
} }
const result = await commandToExecute.action( const result = await commandToExecute.action(
fullCommandContext, fullCommandContext,
args, args,
@ -487,18 +493,8 @@ export const useSlashCommandProcessor = (
content: `Unknown command: ${trimmed}`, content: `Unknown command: ${trimmed}`,
timestamp: new Date(), timestamp: new Date(),
}); });
return { type: 'handled' }; return { type: 'handled' };
} catch (e: unknown) { } catch (e) {
hasError = true;
if (config) {
const event = makeSlashCommandEvent({
command: resolvedCommandPath[0],
subcommand,
status: SlashCommandStatus.ERROR,
});
logSlashCommand(config, event);
}
addItem( addItem(
{ {
type: MessageType.ERROR, type: MessageType.ERROR,
@ -508,14 +504,6 @@ export const useSlashCommandProcessor = (
); );
return { type: 'handled' }; return { type: 'handled' };
} finally { } finally {
if (config && resolvedCommandPath[0] && !hasError) {
const event = makeSlashCommandEvent({
command: resolvedCommandPath[0],
subcommand,
status: SlashCommandStatus.SUCCESS,
});
logSlashCommand(config, event);
}
setIsProcessing(false); setIsProcessing(false);
} }
}, },

View File

@ -15,4 +15,3 @@ export {
IdeConnectionEvent, IdeConnectionEvent,
IdeConnectionType, IdeConnectionType,
} from './src/telemetry/types.js'; } from './src/telemetry/types.js';
export { makeFakeConfig } from './src/test-utils/config.js';

View File

@ -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.enqueueLogEvent(this.createLogEvent(slash_command_event_name, data));
this.flushIfNeeded(); this.flushIfNeeded();
} }

View File

@ -174,9 +174,6 @@ export enum EventMetadataKey {
// Logs the subcommand of the slash command. // Logs the subcommand of the slash command.
GEMINI_CLI_SLASH_COMMAND_SUBCOMMAND = 42, 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 // Next Speaker Check Event Keys
// =========================================================================== // ===========================================================================

View File

@ -41,8 +41,6 @@ export {
FlashFallbackEvent, FlashFallbackEvent,
SlashCommandEvent, SlashCommandEvent,
KittySequenceOverflowEvent, KittySequenceOverflowEvent,
makeSlashCommandEvent,
SlashCommandStatus,
} from './types.js'; } from './types.js';
export { SpanStatusCode, ValueType } from '@opentelemetry/api'; export { SpanStatusCode, ValueType } from '@opentelemetry/api';
export { SemanticAttributes } from '@opentelemetry/semantic-conventions'; export { SemanticAttributes } from '@opentelemetry/semantic-conventions';

View File

@ -14,17 +14,9 @@ import {
ToolCallDecision, ToolCallDecision,
} from './tool-call-decision.js'; } from './tool-call-decision.js';
interface BaseTelemetryEvent { export class StartSessionEvent {
'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.name': 'cli_config';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
model: string; model: string;
embedding_model: string; embedding_model: string;
sandbox_enabled: boolean; 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.name': 'end_session';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
session_id?: string; session_id?: string;
constructor(config?: Config) { 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.name': 'user_prompt';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
prompt_length: number; prompt_length: number;
prompt_id: string; prompt_id: string;
auth_type?: 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.name': 'tool_call';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
function_name: string; function_name: string;
function_args: Record<string, unknown>; function_args: Record<string, unknown>;
duration_ms: number; 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.name': 'api_request';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
model: string; model: string;
prompt_id: string; prompt_id: string;
request_text?: 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.name': 'api_error';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
model: string; model: string;
error: string; error: string;
error_type?: 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.name': 'api_response';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
model: string; model: string;
status_code?: number | string; status_code?: number | string;
duration_ms: number; 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.name': 'flash_fallback';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
auth_type: string; auth_type: string;
constructor(auth_type: string) { constructor(auth_type: string) {
@ -260,9 +252,9 @@ export enum LoopType {
LLM_DETECTED_LOOP = 'llm_detected_loop', LLM_DETECTED_LOOP = 'llm_detected_loop',
} }
export class LoopDetectedEvent implements BaseTelemetryEvent { export class LoopDetectedEvent {
'event.name': 'loop_detected'; 'event.name': 'loop_detected';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
loop_type: LoopType; loop_type: LoopType;
prompt_id: string; 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.name': 'next_speaker_check';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
prompt_id: string; prompt_id: string;
finish_reason: string; finish_reason: string;
result: 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.name': 'slash_command';
'event.timestamp': string; // ISO 8106 'event.timestamp': string; // ISO 8106
command: string; command: string;
subcommand?: 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({ export class MalformedJsonResponseEvent {
command,
subcommand,
status,
}: Omit<SlashCommandEvent, CommonFields>): 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.name': 'malformed_json_response';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
model: string; model: string;
constructor(model: string) { constructor(model: string) {
@ -336,7 +315,7 @@ export enum IdeConnectionType {
export class IdeConnectionEvent { export class IdeConnectionEvent {
'event.name': 'ide_connection'; 'event.name': 'ide_connection';
'event.timestamp': string; 'event.timestamp': string; // ISO 8601
connection_type: IdeConnectionType; connection_type: IdeConnectionType;
constructor(connection_type: IdeConnectionType) { constructor(connection_type: IdeConnectionType) {