Fix extensions logging race condition and slash command logging (#12732)

This commit is contained in:
christine betts
2025-11-08 10:29:36 -05:00
committed by GitHub
parent 9116cf2bab
commit 43b8731241
6 changed files with 105 additions and 51 deletions
+9 -7
View File
@@ -303,7 +303,7 @@ export class ExtensionManager extends ExtensionLoader {
throw new Error(`Extension not found`);
}
if (isUpdate) {
logExtensionUpdateEvent(
await logExtensionUpdateEvent(
this.telemetryConfig,
new ExtensionUpdateEvent(
hashValue(newExtensionConfig.name),
@@ -315,7 +315,7 @@ export class ExtensionManager extends ExtensionLoader {
),
);
} else {
logExtensionInstallEvent(
await logExtensionInstallEvent(
this.telemetryConfig,
new ExtensionInstallEvent(
hashValue(newExtensionConfig.name),
@@ -348,7 +348,7 @@ export class ExtensionManager extends ExtensionLoader {
? getExtensionId(config, installMetadata)
: undefined;
if (isUpdate) {
logExtensionUpdateEvent(
await logExtensionUpdateEvent(
this.telemetryConfig,
new ExtensionUpdateEvent(
hashValue(config?.name ?? ''),
@@ -360,7 +360,7 @@ export class ExtensionManager extends ExtensionLoader {
),
);
} else {
logExtensionInstallEvent(
await logExtensionInstallEvent(
this.telemetryConfig,
new ExtensionInstallEvent(
hashValue(newExtensionConfig?.name ?? ''),
@@ -403,7 +403,7 @@ export class ExtensionManager extends ExtensionLoader {
this.extensionEnablementManager.remove(extension.name);
logExtensionUninstall(
await logExtensionUninstall(
this.telemetryConfig,
new ExtensionUninstallEvent(
hashValue(extension.name),
@@ -569,6 +569,8 @@ export class ExtensionManager extends ExtensionLoader {
const status = workspaceEnabled ? chalk.green('✓') : chalk.red('✗');
let output = `${status} ${extension.name} (${extension.version})`;
output += `\n ID: ${extension.id}`;
output += `\n name: ${hashValue(extension.name)}`;
output += `\n Path: ${extension.path}`;
if (extension.installMetadata) {
output += `\n Source: ${extension.installMetadata.source} (Type: ${extension.installMetadata.type})`;
@@ -621,7 +623,7 @@ export class ExtensionManager extends ExtensionLoader {
scope === SettingScope.Workspace ? this.workspaceDir : os.homedir();
this.extensionEnablementManager.disable(name, true, scopePath);
}
logExtensionDisable(
await logExtensionDisable(
this.telemetryConfig,
new ExtensionDisableEvent(hashValue(name), extension.id, scope),
);
@@ -656,7 +658,7 @@ export class ExtensionManager extends ExtensionLoader {
scope === SettingScope.Workspace ? this.workspaceDir : os.homedir();
this.extensionEnablementManager.enable(name, true, scopePath);
}
logExtensionEnable(
await logExtensionEnable(
this.telemetryConfig,
new ExtensionEnableEvent(hashValue(name), extension.id, scope),
);
@@ -842,6 +842,52 @@ describe('FileCommandLoader', () => {
assert.fail('Incorrect action type');
}
});
it('correctly loads extensionId for extension commands', async () => {
const extensionId = 'my-test-ext-id-123';
const extensionDir = path.join(
process.cwd(),
GEMINI_DIR,
'extensions',
'my-test-ext',
);
mock({
[extensionDir]: {
'gemini-extension.json': JSON.stringify({
name: 'my-test-ext',
id: extensionId,
version: '1.0.0',
}),
commands: {
'my-cmd.toml': 'prompt = "My test command"',
},
},
});
const mockConfig = {
getProjectRoot: vi.fn(() => process.cwd()),
getExtensions: vi.fn(() => [
{
name: 'my-test-ext',
id: extensionId,
version: '1.0.0',
isActive: true,
path: extensionDir,
},
]),
getFolderTrust: vi.fn(() => false),
isTrustedFolder: vi.fn(() => false),
} as unknown as Config;
const loader = new FileCommandLoader(mockConfig);
const commands = await loader.loadCommands(signal);
expect(commands).toHaveLength(1);
const command = commands[0];
expect(command.name).toBe('my-cmd');
expect(command.extensionName).toBe('my-test-ext');
expect(command.extensionId).toBe(extensionId);
});
});
describe('Argument Handling Integration (via ShellProcessor)', () => {
@@ -37,6 +37,7 @@ import { AtFileProcessor } from './prompt-processors/atFileProcessor.js';
interface CommandDirectory {
path: string;
extensionName?: string;
extensionId?: string;
}
/**
@@ -110,6 +111,7 @@ export class FileCommandLoader implements ICommandLoader {
path.join(dirInfo.path, file),
dirInfo.path,
dirInfo.extensionName,
dirInfo.extensionId,
),
);
@@ -158,6 +160,7 @@ export class FileCommandLoader implements ICommandLoader {
const extensionCommandDirs = activeExtensions.map((ext) => ({
path: path.join(ext.path, 'commands'),
extensionName: ext.name,
extensionId: ext.id,
}));
dirs.push(...extensionCommandDirs);
@@ -177,6 +180,7 @@ export class FileCommandLoader implements ICommandLoader {
filePath: string,
baseDir: string,
extensionName?: string,
extensionId?: string,
): Promise<SlashCommand | null> {
let fileContent: string;
try {
@@ -265,6 +269,7 @@ export class FileCommandLoader implements ICommandLoader {
description,
kind: CommandKind.FILE,
extensionName,
extensionId,
action: async (
context: CommandContext,
_args: string,
@@ -940,7 +940,7 @@ export class ClearcutLogger {
this.flushIfNeeded();
}
logExtensionInstallEvent(event: ExtensionInstallEvent): void {
async logExtensionInstallEvent(event: ExtensionInstallEvent): Promise<void> {
const data: EventValue[] = [
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_EXTENSION_NAME,
@@ -967,12 +967,14 @@ export class ClearcutLogger {
this.enqueueLogEvent(
this.createBasicLogEvent(EventNames.EXTENSION_INSTALL, data),
);
this.flushToClearcut().catch((error) => {
await this.flushToClearcut().catch((error) => {
debugLogger.debug('Error flushing to Clearcut:', error);
});
}
logExtensionUninstallEvent(event: ExtensionUninstallEvent): void {
async logExtensionUninstallEvent(
event: ExtensionUninstallEvent,
): Promise<void> {
const data: EventValue[] = [
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_EXTENSION_NAME,
@@ -991,12 +993,12 @@ export class ClearcutLogger {
this.enqueueLogEvent(
this.createBasicLogEvent(EventNames.EXTENSION_UNINSTALL, data),
);
this.flushToClearcut().catch((error) => {
await this.flushToClearcut().catch((error) => {
debugLogger.debug('Error flushing to Clearcut:', error);
});
}
logExtensionUpdateEvent(event: ExtensionUpdateEvent): void {
async logExtensionUpdateEvent(event: ExtensionUpdateEvent): Promise<void> {
const data: EventValue[] = [
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_EXTENSION_NAME,
@@ -1027,7 +1029,7 @@ export class ClearcutLogger {
this.enqueueLogEvent(
this.createBasicLogEvent(EventNames.EXTENSION_UPDATE, data),
);
this.flushToClearcut().catch((error) => {
await this.flushToClearcut().catch((error) => {
debugLogger.debug('Error flushing to Clearcut:', error);
});
}
@@ -1096,7 +1098,7 @@ export class ClearcutLogger {
this.flushIfNeeded();
}
logExtensionEnableEvent(event: ExtensionEnableEvent): void {
async logExtensionEnableEvent(event: ExtensionEnableEvent): Promise<void> {
const data: EventValue[] = [
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_EXTENSION_NAME,
@@ -1116,7 +1118,7 @@ export class ClearcutLogger {
this.enqueueLogEvent(
this.createBasicLogEvent(EventNames.EXTENSION_ENABLE, data),
);
this.flushToClearcut().catch((error) => {
await this.flushToClearcut().catch((error) => {
debugLogger.debug('Error flushing to Clearcut:', error);
});
}
@@ -1135,7 +1137,7 @@ export class ClearcutLogger {
this.flushIfNeeded();
}
logExtensionDisableEvent(event: ExtensionDisableEvent): void {
async logExtensionDisableEvent(event: ExtensionDisableEvent): Promise<void> {
const data: EventValue[] = [
{
gemini_cli_key: EventMetadataKey.GEMINI_CLI_EXTENSION_NAME,
@@ -1155,7 +1157,7 @@ export class ClearcutLogger {
this.enqueueLogEvent(
this.createBasicLogEvent(EventNames.EXTENSION_DISABLE, data),
);
this.flushToClearcut().catch((error) => {
await this.flushToClearcut().catch((error) => {
debugLogger.debug('Error flushing to Clearcut:', error);
});
}
+18 -19
View File
@@ -1516,10 +1516,10 @@ describe('loggers', () => {
});
afterEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
});
it('should log extension install event', () => {
it('should log extension install event', async () => {
const event = new ExtensionInstallEvent(
'testing',
'testing-id',
@@ -1528,7 +1528,7 @@ describe('loggers', () => {
'success',
);
logExtensionInstallEvent(mockConfig, event);
await logExtensionInstallEvent(mockConfig, event);
expect(
ClearcutLogger.prototype.logExtensionInstallEvent,
@@ -1551,7 +1551,7 @@ describe('loggers', () => {
});
});
describe('logExtensionUpdate', () => {
describe('logExtensionUpdate', async () => {
const mockConfig = {
getSessionId: () => 'test-session-id',
getUsageStatisticsEnabled: () => true,
@@ -1565,10 +1565,10 @@ describe('loggers', () => {
});
afterEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
});
it('should log extension update event', () => {
it('should log extension update event', async () => {
const event = new ExtensionUpdateEvent(
'testing',
'testing-id',
@@ -1578,7 +1578,7 @@ describe('loggers', () => {
'success',
);
logExtensionUpdateEvent(mockConfig, event);
await logExtensionUpdateEvent(mockConfig, event);
expect(
ClearcutLogger.prototype.logExtensionUpdateEvent,
@@ -1602,7 +1602,7 @@ describe('loggers', () => {
});
});
describe('logExtensionUninstall', () => {
describe('logExtensionUninstall', async () => {
const mockConfig = {
getSessionId: () => 'test-session-id',
getUsageStatisticsEnabled: () => true,
@@ -1616,17 +1616,16 @@ describe('loggers', () => {
});
afterEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
});
it('should log extension uninstall event', () => {
it('should log extension uninstall event', async () => {
const event = new ExtensionUninstallEvent(
'testing',
'testing-id',
'success',
);
logExtensionUninstall(mockConfig, event);
await logExtensionUninstall(mockConfig, event);
expect(
ClearcutLogger.prototype.logExtensionUninstallEvent,
@@ -1647,7 +1646,7 @@ describe('loggers', () => {
});
});
describe('logExtensionEnable', () => {
describe('logExtensionEnable', async () => {
const mockConfig = {
getSessionId: () => 'test-session-id',
getUsageStatisticsEnabled: () => true,
@@ -1658,13 +1657,13 @@ describe('loggers', () => {
});
afterEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
});
it('should log extension enable event', () => {
it('should log extension enable event', async () => {
const event = new ExtensionEnableEvent('testing', 'testing-id', 'user');
logExtensionEnable(mockConfig, event);
await logExtensionEnable(mockConfig, event);
expect(
ClearcutLogger.prototype.logExtensionEnableEvent,
@@ -1696,13 +1695,13 @@ describe('loggers', () => {
});
afterEach(() => {
vi.resetAllMocks();
vi.clearAllMocks();
});
it('should log extension disable event', () => {
it('should log extension disable event', async () => {
const event = new ExtensionDisableEvent('testing', 'testing-id', 'user');
logExtensionDisable(mockConfig, event);
await logExtensionDisable(mockConfig, event);
expect(
ClearcutLogger.prototype.logExtensionDisableEvent,
+15 -15
View File
@@ -507,11 +507,11 @@ export function logModelSlashCommand(
recordModelSlashCommand(config, event);
}
export function logExtensionInstallEvent(
export async function logExtensionInstallEvent(
config: Config,
event: ExtensionInstallEvent,
): void {
ClearcutLogger.getInstance(config)?.logExtensionInstallEvent(event);
): Promise<void> {
await ClearcutLogger.getInstance(config)?.logExtensionInstallEvent(event);
if (!isTelemetrySdkInitialized()) return;
const logger = logs.getLogger(SERVICE_NAME);
@@ -522,11 +522,11 @@ export function logExtensionInstallEvent(
logger.emit(logRecord);
}
export function logExtensionUninstall(
export async function logExtensionUninstall(
config: Config,
event: ExtensionUninstallEvent,
): void {
ClearcutLogger.getInstance(config)?.logExtensionUninstallEvent(event);
): Promise<void> {
await ClearcutLogger.getInstance(config)?.logExtensionUninstallEvent(event);
if (!isTelemetrySdkInitialized()) return;
const logger = logs.getLogger(SERVICE_NAME);
@@ -537,11 +537,11 @@ export function logExtensionUninstall(
logger.emit(logRecord);
}
export function logExtensionUpdateEvent(
export async function logExtensionUpdateEvent(
config: Config,
event: ExtensionUpdateEvent,
): void {
ClearcutLogger.getInstance(config)?.logExtensionUpdateEvent(event);
): Promise<void> {
await ClearcutLogger.getInstance(config)?.logExtensionUpdateEvent(event);
if (!isTelemetrySdkInitialized()) return;
const logger = logs.getLogger(SERVICE_NAME);
@@ -552,11 +552,11 @@ export function logExtensionUpdateEvent(
logger.emit(logRecord);
}
export function logExtensionEnable(
export async function logExtensionEnable(
config: Config,
event: ExtensionEnableEvent,
): void {
ClearcutLogger.getInstance(config)?.logExtensionEnableEvent(event);
): Promise<void> {
await ClearcutLogger.getInstance(config)?.logExtensionEnableEvent(event);
if (!isTelemetrySdkInitialized()) return;
const logger = logs.getLogger(SERVICE_NAME);
@@ -567,11 +567,11 @@ export function logExtensionEnable(
logger.emit(logRecord);
}
export function logExtensionDisable(
export async function logExtensionDisable(
config: Config,
event: ExtensionDisableEvent,
): void {
ClearcutLogger.getInstance(config)?.logExtensionDisableEvent(event);
): Promise<void> {
await ClearcutLogger.getInstance(config)?.logExtensionDisableEvent(event);
if (!isTelemetrySdkInitialized()) return;
const logger = logs.getLogger(SERVICE_NAME);