From d47d4855dbf7747aeb1143739254153b61f0e7f5 Mon Sep 17 00:00:00 2001 From: Michael Bleigh Date: Tue, 24 Feb 2026 13:03:36 -0800 Subject: [PATCH] feat(hooks): adds support for RuntimeHook functions. (#19598) --- .../extension-manager-hydration.test.ts | 14 +- packages/cli/src/config/extension-manager.ts | 7 +- packages/core/src/config/config.test.ts | 4 +- packages/core/src/hooks/hookEventHandler.ts | 9 +- packages/core/src/hooks/hookRegistry.test.ts | 13 +- packages/core/src/hooks/hookRegistry.ts | 50 ++++++- packages/core/src/hooks/hookRunner.ts | 75 +++++++++- packages/core/src/hooks/hookSystem.test.ts | 11 +- packages/core/src/hooks/hookSystem.ts | 14 ++ packages/core/src/hooks/runtimeHooks.test.ts | 141 ++++++++++++++++++ packages/core/src/hooks/trustedHooks.test.ts | 43 ++++-- packages/core/src/hooks/trustedHooks.ts | 3 + packages/core/src/hooks/types.ts | 46 ++++-- .../clearcut-logger/clearcut-logger.test.ts | 3 +- packages/core/src/telemetry/loggers.test.ts | 3 +- packages/core/src/telemetry/sanitize.test.ts | 37 ++--- packages/core/src/telemetry/types.ts | 5 +- 17 files changed, 410 insertions(+), 68 deletions(-) create mode 100644 packages/core/src/hooks/runtimeHooks.test.ts diff --git a/packages/cli/src/config/extension-manager-hydration.test.ts b/packages/cli/src/config/extension-manager-hydration.test.ts index def5fed42c..ff266976a5 100644 --- a/packages/cli/src/config/extension-manager-hydration.test.ts +++ b/packages/cli/src/config/extension-manager-hydration.test.ts @@ -9,7 +9,11 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; import { ExtensionManager } from './extension-manager.js'; -import { debugLogger, coreEvents } from '@google/gemini-cli-core'; +import { + debugLogger, + coreEvents, + type CommandHookConfig, +} from '@google/gemini-cli-core'; import { createTestMergedSettings } from './settings.js'; import { createExtension } from '../test-utils/createExtension.js'; import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js'; @@ -248,9 +252,11 @@ System using model: \${MODEL_NAME} expect(extension.hooks).toBeDefined(); expect(extension.hooks?.BeforeTool).toHaveLength(1); - expect(extension.hooks?.BeforeTool![0].hooks[0].env?.['HOOK_CMD']).toBe( - 'hello-world', - ); + expect( + (extension.hooks?.BeforeTool![0].hooks[0] as CommandHookConfig).env?.[ + 'HOOK_CMD' + ], + ).toBe('hello-world'); }); it('should pick up new settings after restartExtension', async () => { diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 805178ac70..8d3b5fa15f 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -52,6 +52,7 @@ import { applyAdminAllowlist, getAdminBlockedMcpServersMessage, CoreToolCallStatus, + HookType, } from '@google/gemini-cli-core'; import { maybeRequestConsentOrFail } from './extensions/consent.js'; import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; @@ -735,8 +736,10 @@ Would you like to attempt to install via "git clone" instead?`, if (eventHooks) { for (const definition of eventHooks) { for (const hook of definition.hooks) { - // Merge existing env with new env vars, giving extension settings precedence. - hook.env = { ...hook.env, ...hookEnv }; + if (hook.type === HookType.Command) { + // Merge existing env with new env vars, giving extension settings precedence. + hook.env = { ...hook.env, ...hookEnv }; + } } } } diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 0346cc1831..1a8f695bd7 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -1928,7 +1928,7 @@ describe('Config getHooks', () => { const mockHooks = { BeforeTool: [ { - hooks: [{ type: HookType.Command, command: 'echo 1' }], + hooks: [{ type: HookType.Command, command: 'echo 1' } as const], }, ], }; @@ -2235,7 +2235,7 @@ describe('Hooks configuration', () => { const initialHooks = { BeforeAgent: [ { - hooks: [{ type: HookType.Command, command: 'initial' }], + hooks: [{ type: HookType.Command as const, command: 'initial' }], }, ], }; diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index 0e744c3be7..00909094ce 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -8,7 +8,7 @@ import type { Config } from '../config/config.js'; import type { HookPlanner, HookEventContext } from './hookPlanner.js'; import type { HookRunner } from './hookRunner.js'; import type { HookAggregator, AggregatedHookResult } from './hookAggregator.js'; -import { HookEventName } from './types.js'; +import { HookEventName, HookType } from './types.js'; import type { HookConfig, HookInput, @@ -500,7 +500,10 @@ export class HookEventHandler { * Get hook name from config for display or telemetry */ private getHookName(config: HookConfig): string { - return config.name || config.command || 'unknown-command'; + if (config.type === HookType.Command) { + return config.name || config.command || 'unknown-command'; + } + return config.name || 'unknown-hook'; } /** @@ -513,7 +516,7 @@ export class HookEventHandler { /** * Get hook type from execution result for telemetry */ - private getHookTypeFromResult(result: HookExecutionResult): 'command' { + private getHookTypeFromResult(result: HookExecutionResult): HookType { return result.hookConfig.type; } } diff --git a/packages/core/src/hooks/hookRegistry.test.ts b/packages/core/src/hooks/hookRegistry.test.ts index 0308eae70a..078990608c 100644 --- a/packages/core/src/hooks/hookRegistry.test.ts +++ b/packages/core/src/hooks/hookRegistry.test.ts @@ -13,6 +13,7 @@ import { HookEventName, HookType, HOOKS_CONFIG_FIELDS, + type CommandHookConfig, } from './types.js'; import type { Config } from '../config/config.js'; import type { HookDefinition } from './types.js'; @@ -153,7 +154,9 @@ describe('HookRegistry', () => { expect(hooks).toHaveLength(1); expect(hooks[0].eventName).toBe(HookEventName.BeforeTool); expect(hooks[0].config.type).toBe(HookType.Command); - expect(hooks[0].config.command).toBe('./hooks/check_style.sh'); + expect((hooks[0].config as CommandHookConfig).command).toBe( + './hooks/check_style.sh', + ); expect(hooks[0].matcher).toBe('EditTool'); expect(hooks[0].source).toBe(ConfigSource.Project); }); @@ -186,7 +189,9 @@ describe('HookRegistry', () => { expect(hooks).toHaveLength(1); expect(hooks[0].eventName).toBe(HookEventName.AfterTool); expect(hooks[0].config.type).toBe(HookType.Command); - expect(hooks[0].config.command).toBe('./hooks/after-tool.sh'); + expect((hooks[0].config as CommandHookConfig).command).toBe( + './hooks/after-tool.sh', + ); }); it('should handle invalid configuration gracefully', async () => { @@ -632,7 +637,9 @@ describe('HookRegistry', () => { // Should only load the valid hook const hooks = hookRegistry.getAllHooks(); expect(hooks).toHaveLength(1); - expect(hooks[0].config.command).toBe('./valid-hook.sh'); + expect((hooks[0].config as CommandHookConfig).command).toBe( + './valid-hook.sh', + ); // Verify the warnings for invalid configurations // 1st warning: non-object hookConfig ('invalid-string') diff --git a/packages/core/src/hooks/hookRegistry.ts b/packages/core/src/hooks/hookRegistry.ts index 8ae142231a..b76478d152 100644 --- a/packages/core/src/hooks/hookRegistry.ts +++ b/packages/core/src/hooks/hookRegistry.ts @@ -34,11 +34,40 @@ export class HookRegistry { this.config = config; } + /** + * Register a new hook programmatically + */ + registerHook( + config: HookConfig, + eventName: HookEventName, + options?: { matcher?: string; sequential?: boolean; source?: ConfigSource }, + ): void { + const source = options?.source ?? ConfigSource.Runtime; + + if (!this.validateHookConfig(config, eventName, source)) { + throw new Error( + `Invalid hook configuration for ${eventName} from ${source}`, + ); + } + + this.entries.push({ + config, + source, + eventName, + matcher: options?.matcher, + sequential: options?.sequential, + enabled: true, + }); + } + /** * Initialize the registry by processing hooks from config */ async initialize(): Promise { - this.entries = []; + const runtimeHooks = this.entries.filter( + (entry) => entry.source === ConfigSource.Runtime, + ); + this.entries = [...runtimeHooks]; this.processHooksFromConfig(); debugLogger.debug( @@ -93,7 +122,10 @@ export class HookRegistry { private getHookName( entry: HookRegistryEntry | { config: HookConfig }, ): string { - return entry.config.name || entry.config.command || 'unknown-command'; + if (entry.config.type === 'command') { + return entry.config.name || entry.config.command || 'unknown-command'; + } + return entry.config.name || 'unknown-hook'; } /** @@ -261,7 +293,10 @@ please review the project settings (.gemini/settings.json) and remove them.`; eventName: HookEventName, source: ConfigSource, ): boolean { - if (!config.type || !['command', 'plugin'].includes(config.type)) { + if ( + !config.type || + !['command', 'plugin', 'runtime'].includes(config.type) + ) { debugLogger.warn( `Invalid hook ${eventName} from ${source} type: ${config.type}`, ); @@ -275,6 +310,13 @@ please review the project settings (.gemini/settings.json) and remove them.`; return false; } + if (config.type === 'runtime' && !config.name) { + debugLogger.warn( + `Runtime hook ${eventName} from ${source} missing name field`, + ); + return false; + } + return true; } @@ -292,6 +334,8 @@ please review the project settings (.gemini/settings.json) and remove them.`; */ private getSourcePriority(source: ConfigSource): number { switch (source) { + case ConfigSource.Runtime: + return 0; // Highest case ConfigSource.Project: return 1; case ConfigSource.User: diff --git a/packages/core/src/hooks/hookRunner.ts b/packages/core/src/hooks/hookRunner.ts index 05f81d1983..f608a349f9 100644 --- a/packages/core/src/hooks/hookRunner.ts +++ b/packages/core/src/hooks/hookRunner.ts @@ -7,6 +7,8 @@ import { spawn } from 'node:child_process'; import type { HookConfig, + CommandHookConfig, + RuntimeHookConfig, HookInput, HookOutput, HookExecutionResult, @@ -15,7 +17,7 @@ import type { BeforeModelOutput, BeforeToolInput, } from './types.js'; -import { HookEventName, ConfigSource } from './types.js'; +import { HookEventName, ConfigSource, HookType } from './types.js'; import type { Config } from '../config/config.js'; import type { LLMRequest } from './hookTranslator.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -75,6 +77,15 @@ export class HookRunner { } try { + if (hookConfig.type === HookType.Runtime) { + return await this.executeRuntimeHook( + hookConfig, + eventName, + input, + startTime, + ); + } + return await this.executeCommandHook( hookConfig, eventName, @@ -83,7 +94,10 @@ export class HookRunner { ); } catch (error) { const duration = Date.now() - startTime; - const hookId = hookConfig.name || hookConfig.command || 'unknown'; + const hookId = + hookConfig.name || + (hookConfig.type === HookType.Command ? hookConfig.command : '') || + 'unknown'; const errorMessage = `Hook execution failed for event '${eventName}' (hook: ${hookId}): ${error}`; debugLogger.warn(`Hook execution error (non-fatal): ${errorMessage}`); @@ -230,11 +244,66 @@ export class HookRunner { return modifiedInput; } + /** + * Execute a runtime hook + */ + private async executeRuntimeHook( + hookConfig: RuntimeHookConfig, + eventName: HookEventName, + input: HookInput, + startTime: number, + ): Promise { + const timeout = hookConfig.timeout ?? DEFAULT_HOOK_TIMEOUT; + let timeoutHandle: ReturnType | undefined; + const controller = new AbortController(); + + try { + // Create a promise that rejects after timeout + const timeoutPromise = new Promise((_, reject) => { + timeoutHandle = setTimeout( + () => reject(new Error(`Hook timed out after ${timeout}ms`)), + timeout, + ); + }); + + // Execute action with timeout race + const result = await Promise.race([ + hookConfig.action(input, { signal: controller.signal }), + timeoutPromise, + ]); + + const output = + result === null || result === undefined ? undefined : result; + + return { + hookConfig, + eventName, + success: true, + output, + duration: Date.now() - startTime, + }; + } catch (error) { + // Abort the ongoing hook action if it timed out or errored + controller.abort(); + return { + hookConfig, + eventName, + success: false, + error: error instanceof Error ? error : new Error(String(error)), + duration: Date.now() - startTime, + }; + } finally { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + } + } + /** * Execute a command hook */ private async executeCommandHook( - hookConfig: HookConfig, + hookConfig: CommandHookConfig, eventName: HookEventName, input: HookInput, startTime: number, diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts index 34bc687c5b..85f1a7407b 100644 --- a/packages/core/src/hooks/hookSystem.test.ts +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -77,7 +77,7 @@ describe('HookSystem Integration', () => { matcher: 'TestTool', hooks: [ { - type: HookType.Command, + type: HookType.Command as const, command: 'echo', timeout: 5000, }, @@ -164,7 +164,8 @@ describe('HookSystem Integration', () => { { type: 'invalid-type' as HookType, // Invalid hook type for testing command: './test.sh', - }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, ], }, ], @@ -279,12 +280,12 @@ describe('HookSystem Integration', () => { matcher: 'TestTool', hooks: [ { - type: HookType.Command, + type: HookType.Command as const, command: 'echo "enabled-hook"', timeout: 5000, }, { - type: HookType.Command, + type: HookType.Command as const, command: 'echo "disabled-hook"', timeout: 5000, }, @@ -350,7 +351,7 @@ describe('HookSystem Integration', () => { matcher: 'TestTool', hooks: [ { - type: HookType.Command, + type: HookType.Command as const, command: 'echo "will-be-disabled"', timeout: 5000, }, diff --git a/packages/core/src/hooks/hookSystem.ts b/packages/core/src/hooks/hookSystem.ts index 56eb10b015..84494ee1ea 100644 --- a/packages/core/src/hooks/hookSystem.ts +++ b/packages/core/src/hooks/hookSystem.ts @@ -21,6 +21,9 @@ import type { AfterModelHookOutput, BeforeToolSelectionHookOutput, McpToolContext, + HookConfig, + HookEventName, + ConfigSource, } from './types.js'; import { NotificationType } from './types.js'; import type { AggregatedHookResult } from './hookAggregator.js'; @@ -202,6 +205,17 @@ export class HookSystem { return this.hookRegistry.getAllHooks(); } + /** + * Register a new hook programmatically + */ + registerHook( + config: HookConfig, + eventName: HookEventName, + options?: { matcher?: string; sequential?: boolean; source?: ConfigSource }, + ): void { + this.hookRegistry.registerHook(config, eventName, options); + } + /** * Fire hook events directly */ diff --git a/packages/core/src/hooks/runtimeHooks.test.ts b/packages/core/src/hooks/runtimeHooks.test.ts new file mode 100644 index 0000000000..970eaca397 --- /dev/null +++ b/packages/core/src/hooks/runtimeHooks.test.ts @@ -0,0 +1,141 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HookSystem } from './hookSystem.js'; +import { Config } from '../config/config.js'; +import { HookType, HookEventName, ConfigSource } from './types.js'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import * as fs from 'node:fs'; + +// Mock console methods +vi.stubGlobal('console', { + log: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), +}); + +describe('Runtime Hooks', () => { + let hookSystem: HookSystem; + let config: Config; + + beforeEach(() => { + vi.resetAllMocks(); + const testDir = path.join(os.tmpdir(), 'test-runtime-hooks'); + fs.mkdirSync(testDir, { recursive: true }); + + config = new Config({ + model: 'gemini-3-flash-preview', + targetDir: testDir, + sessionId: 'test-session', + debugMode: false, + cwd: testDir, + }); + + // Stub getMessageBus + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (config as any).getMessageBus = () => undefined; + + hookSystem = new HookSystem(config); + }); + + it('should register a runtime hook', async () => { + await hookSystem.initialize(); + + const action = vi.fn().mockResolvedValue(undefined); + hookSystem.registerHook( + { + type: HookType.Runtime, + name: 'test-hook', + action, + }, + HookEventName.BeforeTool, + { matcher: 'TestTool' }, + ); + + const hooks = hookSystem.getAllHooks(); + expect(hooks).toHaveLength(1); + expect(hooks[0].config.name).toBe('test-hook'); + expect(hooks[0].source).toBe(ConfigSource.Runtime); + }); + + it('should execute a runtime hook', async () => { + await hookSystem.initialize(); + + const action = vi.fn().mockImplementation(async () => ({ + decision: 'allow', + systemMessage: 'Hook ran', + })); + + hookSystem.registerHook( + { + type: HookType.Runtime, + name: 'test-hook', + action, + }, + HookEventName.BeforeTool, + { matcher: 'TestTool' }, + ); + + const result = await hookSystem + .getEventHandler() + .fireBeforeToolEvent('TestTool', { foo: 'bar' }); + + expect(action).toHaveBeenCalled(); + expect(action.mock.calls[0][0]).toMatchObject({ + tool_name: 'TestTool', + tool_input: { foo: 'bar' }, + hook_event_name: 'BeforeTool', + }); + + expect(result.finalOutput?.systemMessage).toBe('Hook ran'); + }); + + it('should handle runtime hook errors', async () => { + await hookSystem.initialize(); + + const action = vi.fn().mockRejectedValue(new Error('Hook failed')); + + hookSystem.registerHook( + { + type: HookType.Runtime, + name: 'fail-hook', + action, + }, + HookEventName.BeforeTool, + { matcher: 'TestTool' }, + ); + + // Should not throw, but handle error gracefully + await hookSystem.getEventHandler().fireBeforeToolEvent('TestTool', {}); + + expect(action).toHaveBeenCalled(); + }); + + it('should preserve runtime hooks across re-initialization', async () => { + await hookSystem.initialize(); + + hookSystem.registerHook( + { + type: HookType.Runtime, + name: 'persist-hook', + action: async () => {}, + }, + HookEventName.BeforeTool, + { matcher: 'TestTool' }, + ); + + expect(hookSystem.getAllHooks()).toHaveLength(1); + + // Re-initialize + await hookSystem.initialize(); + + expect(hookSystem.getAllHooks()).toHaveLength(1); + expect(hookSystem.getAllHooks()[0].config.name).toBe('persist-hook'); + }); +}); diff --git a/packages/core/src/hooks/trustedHooks.test.ts b/packages/core/src/hooks/trustedHooks.test.ts index b68d4fcd9c..6e32d32d5c 100644 --- a/packages/core/src/hooks/trustedHooks.test.ts +++ b/packages/core/src/hooks/trustedHooks.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import * as fs from 'node:fs'; import { TrustedHooksManager } from './trustedHooks.js'; import { Storage } from '../config/storage.js'; -import { HookEventName, HookType } from './types.js'; +import { HookEventName, HookType, type HookDefinition } from './types.js'; vi.mock('node:fs'); vi.mock('../config/storage.js'); @@ -72,8 +72,16 @@ describe('TrustedHooksManager', () => { [HookEventName.BeforeTool]: [ { hooks: [ - { name: 'trusted-hook', type: HookType.Command, command: 'cmd1' }, - { name: 'new-hook', type: HookType.Command, command: 'cmd2' }, + { + name: 'trusted-hook', + type: HookType.Command, + command: 'cmd1', + } as const, + { + name: 'new-hook', + type: HookType.Command, + command: 'cmd2', + } as const, ], }, ], @@ -90,7 +98,11 @@ describe('TrustedHooksManager', () => { [HookEventName.BeforeTool]: [ { hooks: [ - { name: 'trusted-hook', type: HookType.Command, command: 'cmd1' }, + { + name: 'trusted-hook', + type: HookType.Command, + command: 'cmd1', + } as const, ], }, ], @@ -114,9 +126,12 @@ describe('TrustedHooksManager', () => { ], }; - expect(manager.getUntrustedHooks('/project', projectHooks)).toEqual([ - './script.sh', - ]); + expect( + manager.getUntrustedHooks( + '/project', + projectHooks as Partial>, + ), + ).toEqual(['./script.sh']); }); it('should detect change in command as untrusted', () => { @@ -142,11 +157,17 @@ describe('TrustedHooksManager', () => { ], }; - manager.trustHooks('/project', originalHook); + manager.trustHooks( + '/project', + originalHook as Partial>, + ); - expect(manager.getUntrustedHooks('/project', updatedHook)).toEqual([ - 'my-hook', - ]); + expect( + manager.getUntrustedHooks( + '/project', + updatedHook as Partial>, + ), + ).toEqual(['my-hook']); }); }); diff --git a/packages/core/src/hooks/trustedHooks.ts b/packages/core/src/hooks/trustedHooks.ts index 562f0e76bb..5c8dc2cc78 100644 --- a/packages/core/src/hooks/trustedHooks.ts +++ b/packages/core/src/hooks/trustedHooks.ts @@ -9,6 +9,7 @@ import * as path from 'node:path'; import { Storage } from '../config/storage.js'; import { getHookKey, + HookType, type HookDefinition, type HookEventName, } from './types.js'; @@ -79,6 +80,7 @@ export class TrustedHooksManager { for (const def of definitions) { if (!def || !Array.isArray(def.hooks)) continue; for (const hook of def.hooks) { + if (hook.type === HookType.Runtime) continue; const key = getHookKey(hook); if (!trustedKeys.has(key)) { // Return friendly name or command @@ -108,6 +110,7 @@ export class TrustedHooksManager { for (const def of definitions) { if (!def || !Array.isArray(def.hooks)) continue; for (const hook of def.hooks) { + if (hook.type === HookType.Runtime) continue; currentTrusted.add(getHookKey(hook)); } } diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index ba579d81e6..b053f22f59 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -21,6 +21,7 @@ import { defaultHookTranslator } from './hookTranslator.js'; * Configuration source levels in precedence order (highest to lowest) */ export enum ConfigSource { + Runtime = 'runtime', Project = 'project', User = 'user', System = 'system', @@ -50,11 +51,43 @@ export enum HookEventName { export const HOOKS_CONFIG_FIELDS = ['enabled', 'disabled', 'notifications']; /** - * Hook configuration entry + * Hook implementation types + */ +export enum HookType { + Command = 'command', + Runtime = 'runtime', +} + +/** + * Hook action function + */ +export type HookAction = ( + input: HookInput, + options?: { signal: AbortSignal }, +) => Promise; + +/** + * Runtime hook configuration + */ +export interface RuntimeHookConfig { + type: HookType.Runtime; + /** Unique name for the runtime hook */ + name: string; + /** Function to execute when the hook is triggered */ + action: HookAction; + command?: never; + source?: ConfigSource; + /** Maximum time allowed for hook execution in milliseconds */ + timeout?: number; +} + +/** + * Command hook configuration entry */ export interface CommandHookConfig { type: HookType.Command; command: string; + action?: never; name?: string; description?: string; timeout?: number; @@ -62,7 +95,7 @@ export interface CommandHookConfig { env?: Record; } -export type HookConfig = CommandHookConfig; +export type HookConfig = CommandHookConfig | RuntimeHookConfig; /** * Hook definition with matcher @@ -73,19 +106,12 @@ export interface HookDefinition { hooks: HookConfig[]; } -/** - * Hook implementation types - */ -export enum HookType { - Command = 'command', -} - /** * Generate a unique key for a hook configuration */ export function getHookKey(hook: HookConfig): string { const name = hook.name || ''; - const command = hook.command || ''; + const command = hook.type === HookType.Command ? hook.command : ''; return `${name}:${command}`; } diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts index ed87bb34fc..e75daeb6c9 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.test.ts @@ -35,6 +35,7 @@ import { WebFetchFallbackAttemptEvent, HookCallEvent, } from '../types.js'; +import { HookType } from '../../hooks/types.js'; import { AgentTerminateMode } from '../../agents/types.js'; import { GIT_COMMIT_INFO, CLI_VERSION } from '../../generated/git-commit.js'; import { UserAccountManager } from '../../utils/userAccountManager.js'; @@ -1401,7 +1402,7 @@ describe('ClearcutLogger', () => { const event = new HookCallEvent( 'before-tool', - 'command', + HookType.Command, hookName, {}, // input 150, // duration diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index db0e44be25..13d51a2b8e 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -95,6 +95,7 @@ import { EVENT_HOOK_CALL, LlmRole, } from './types.js'; +import { HookType } from '../hooks/types.js'; import * as metrics from './metrics.js'; import { FileOperation } from './metrics.js'; import * as sdk from './sdk.js'; @@ -2327,7 +2328,7 @@ describe('loggers', () => { it('should log hook call event to Clearcut and OTEL', () => { const event = new HookCallEvent( 'before-tool', - 'command', + HookType.Command, '/path/to/script.sh', { arg: 'val' }, 150, diff --git a/packages/core/src/telemetry/sanitize.test.ts b/packages/core/src/telemetry/sanitize.test.ts index 9e179c7552..c3129295a7 100644 --- a/packages/core/src/telemetry/sanitize.test.ts +++ b/packages/core/src/telemetry/sanitize.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect } from 'vitest'; import { HookCallEvent, EVENT_HOOK_CALL } from './types.js'; +import { HookType } from '../hooks/types.js'; import type { Config } from '../config/config.js'; /** @@ -40,7 +41,7 @@ describe('Telemetry Sanitization', () => { it('should create an event with all fields', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'test-hook', { tool_name: 'ReadFile' }, 100, @@ -69,7 +70,7 @@ describe('Telemetry Sanitization', () => { it('should create an event with minimal fields', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'test-hook', { tool_name: 'ReadFile' }, 100, @@ -90,7 +91,7 @@ describe('Telemetry Sanitization', () => { it('should include all fields when logPrompts is enabled', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, '/path/to/.gemini/hooks/check-secrets.sh --api-key=abc123', { tool_name: 'ReadFile', args: { file: 'secret.txt' } }, 100, @@ -123,7 +124,7 @@ describe('Telemetry Sanitization', () => { it('should include hook_input and hook_output as JSON strings', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'test-hook', { tool_name: 'ReadFile', args: { file: 'test.txt' } }, 100, @@ -154,7 +155,7 @@ describe('Telemetry Sanitization', () => { it('should exclude PII-sensitive fields when logPrompts is disabled', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, '/path/to/.gemini/hooks/check-secrets.sh --api-key=abc123', { tool_name: 'ReadFile', args: { file: 'secret.txt' } }, 100, @@ -232,7 +233,7 @@ describe('Telemetry Sanitization', () => { for (const testCase of testCases) { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, testCase.input, { tool_name: 'ReadFile' }, 100, @@ -248,7 +249,7 @@ describe('Telemetry Sanitization', () => { it('should still include error field even when logPrompts is disabled', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'test-hook', { tool_name: 'ReadFile' }, 100, @@ -276,7 +277,7 @@ describe('Telemetry Sanitization', () => { it('should handle commands with multiple spaces', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'python script.py --arg1 --arg2', {}, 100, @@ -290,7 +291,7 @@ describe('Telemetry Sanitization', () => { it('should handle mixed path separators', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, '/path/to\\mixed\\separators.sh', {}, 100, @@ -304,7 +305,7 @@ describe('Telemetry Sanitization', () => { it('should handle trailing slashes', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, '/path/to/directory/', {}, 100, @@ -320,7 +321,7 @@ describe('Telemetry Sanitization', () => { it('should format success message correctly', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'test-hook', {}, 150, @@ -335,7 +336,7 @@ describe('Telemetry Sanitization', () => { it('should format failure message correctly', () => { const event = new HookCallEvent( 'AfterTool', - 'command', + HookType.Command, 'validation-hook', {}, 75, @@ -354,7 +355,7 @@ describe('Telemetry Sanitization', () => { const event = new HookCallEvent( 'BeforeModel', - 'command', + HookType.Command, '$GEMINI_PROJECT_DIR/.gemini/hooks/add-context.sh', { llm_request: { @@ -394,7 +395,7 @@ describe('Telemetry Sanitization', () => { const event = new HookCallEvent( 'BeforeModel', - 'command', + HookType.Command, '$GEMINI_PROJECT_DIR/.gemini/hooks/add-context.sh', { llm_request: { @@ -438,7 +439,7 @@ describe('Telemetry Sanitization', () => { it('should sanitize commands with API keys', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'curl https://api.example.com -H "Authorization: Bearer sk-abc123xyz"', {}, 100, @@ -452,7 +453,7 @@ describe('Telemetry Sanitization', () => { it('should sanitize commands with database credentials', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'psql postgresql://user:password@localhost/db', {}, 100, @@ -466,7 +467,7 @@ describe('Telemetry Sanitization', () => { it('should sanitize commands with environment variables containing secrets', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'AWS_SECRET_KEY=abc123 aws s3 ls', {}, 100, @@ -480,7 +481,7 @@ describe('Telemetry Sanitization', () => { it('should sanitize Python scripts with file paths', () => { const event = new HookCallEvent( 'BeforeTool', - 'command', + HookType.Command, 'python /home/john.doe/projects/secret-scanner/scan.py --config=/etc/secrets.yml', {}, 100, diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 47837f0620..ccebef242b 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -43,6 +43,7 @@ import { sanitizeHookName } from './sanitize.js'; import { getFileDiffFromResultDisplay } from '../utils/fileDiffUtils.js'; import { LlmRole } from './llmRole.js'; export { LlmRole }; +import type { HookType } from '../hooks/types.js'; export interface BaseTelemetryEvent { 'event.name': string; @@ -2166,7 +2167,7 @@ export class HookCallEvent implements BaseTelemetryEvent { 'event.name': string; 'event.timestamp': string; hook_event_name: string; - hook_type: 'command'; + hook_type: HookType; hook_name: string; hook_input: Record; hook_output?: Record; @@ -2179,7 +2180,7 @@ export class HookCallEvent implements BaseTelemetryEvent { constructor( hookEventName: string, - hookType: 'command', + hookType: HookType, hookName: string, hookInput: Record, durationMs: number,