mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-28 22:14:52 -07:00
feat(core): refactor session learnings as a builtin hook
This commit is contained in:
@@ -74,6 +74,7 @@ describe('HookRegistry', () => {
|
|||||||
getDisabledHooks: vi.fn().mockReturnValue([]),
|
getDisabledHooks: vi.fn().mockReturnValue([]),
|
||||||
isTrustedFolder: vi.fn().mockReturnValue(true),
|
isTrustedFolder: vi.fn().mockReturnValue(true),
|
||||||
getProjectRoot: vi.fn().mockReturnValue('/project'),
|
getProjectRoot: vi.fn().mockReturnValue('/project'),
|
||||||
|
isSessionLearningsEnabled: vi.fn().mockReturnValue(false),
|
||||||
} as unknown as Config;
|
} as unknown as Config;
|
||||||
|
|
||||||
hookRegistry = new HookRegistry(mockConfig);
|
hookRegistry = new HookRegistry(mockConfig);
|
||||||
@@ -279,6 +280,21 @@ describe('HookRegistry', () => {
|
|||||||
hookRegistry.getHooksForEvent(HookEventName.BeforeTool),
|
hookRegistry.getHooksForEvent(HookEventName.BeforeTool),
|
||||||
).toHaveLength(0);
|
).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should register builtin session-learnings hook when enabled', async () => {
|
||||||
|
vi.mocked(mockConfig.isSessionLearningsEnabled).mockReturnValue(true);
|
||||||
|
|
||||||
|
await hookRegistry.initialize();
|
||||||
|
|
||||||
|
const hooks = hookRegistry.getHooksForEvent(HookEventName.SessionEnd);
|
||||||
|
expect(hooks).toHaveLength(1);
|
||||||
|
expect(hooks[0].config.type).toBe(HookType.Builtin);
|
||||||
|
|
||||||
|
expect((hooks[0].config as BuiltinHookConfig).builtin_id).toBe(
|
||||||
|
'session-learnings',
|
||||||
|
);
|
||||||
|
expect(hooks[0].source).toBe(ConfigSource.System);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('getHooksForEvent', () => {
|
describe('getHooksForEvent', () => {
|
||||||
|
|||||||
@@ -6,7 +6,12 @@
|
|||||||
|
|
||||||
import type { Config } from '../config/config.js';
|
import type { Config } from '../config/config.js';
|
||||||
import type { HookDefinition, HookConfig } from './types.js';
|
import type { HookDefinition, HookConfig } from './types.js';
|
||||||
import { HookEventName, ConfigSource, HOOKS_CONFIG_FIELDS } from './types.js';
|
import {
|
||||||
|
HookEventName,
|
||||||
|
ConfigSource,
|
||||||
|
HOOKS_CONFIG_FIELDS,
|
||||||
|
HookType,
|
||||||
|
} from './types.js';
|
||||||
import { debugLogger } from '../utils/debugLogger.js';
|
import { debugLogger } from '../utils/debugLogger.js';
|
||||||
import { TrustedHooksManager } from './trustedHooks.js';
|
import { TrustedHooksManager } from './trustedHooks.js';
|
||||||
import { coreEvents } from '../utils/events.js';
|
import { coreEvents } from '../utils/events.js';
|
||||||
@@ -137,6 +142,8 @@ please review the project settings (.gemini/settings.json) and remove them.`;
|
|||||||
this.checkProjectHooksTrust();
|
this.checkProjectHooksTrust();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
this.registerBuiltinHooks();
|
||||||
|
|
||||||
// Get hooks from the main config (this comes from the merged settings)
|
// Get hooks from the main config (this comes from the merged settings)
|
||||||
const configHooks = this.config.getHooks();
|
const configHooks = this.config.getHooks();
|
||||||
if (configHooks) {
|
if (configHooks) {
|
||||||
@@ -161,6 +168,27 @@ please review the project settings (.gemini/settings.json) and remove them.`;
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Register system-level builtin hooks
|
||||||
|
*/
|
||||||
|
private registerBuiltinHooks(): void {
|
||||||
|
if (this.config.isSessionLearningsEnabled()) {
|
||||||
|
debugLogger.debug('Registering builtin session-learnings hook');
|
||||||
|
this.entries.push({
|
||||||
|
config: {
|
||||||
|
type: HookType.Builtin,
|
||||||
|
builtin_id: 'session-learnings',
|
||||||
|
name: 'session-learnings',
|
||||||
|
description: 'Automatically generate session learning summaries',
|
||||||
|
source: ConfigSource.System,
|
||||||
|
},
|
||||||
|
source: ConfigSource.System,
|
||||||
|
eventName: HookEventName.SessionEnd,
|
||||||
|
enabled: true,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Process hooks configuration and add entries
|
* Process hooks configuration and add entries
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -0,0 +1,109 @@
|
|||||||
|
/**
|
||||||
|
* @license
|
||||||
|
* Copyright 2025 Google LLC
|
||||||
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
|
import { HookRunner } from './hookRunner.js';
|
||||||
|
import {
|
||||||
|
HookEventName,
|
||||||
|
HookType,
|
||||||
|
SessionEndReason,
|
||||||
|
type BuiltinHookConfig,
|
||||||
|
type SessionEndInput,
|
||||||
|
} from './types.js';
|
||||||
|
import type { Config } from '../config/config.js';
|
||||||
|
|
||||||
|
describe('HookRunner (Builtin Hooks)', () => {
|
||||||
|
let hookRunner: HookRunner;
|
||||||
|
let mockConfig: Config;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetAllMocks();
|
||||||
|
|
||||||
|
mockConfig = {
|
||||||
|
sanitizationConfig: {},
|
||||||
|
isSessionLearningsEnabled: vi.fn().mockReturnValue(true),
|
||||||
|
getGeminiClient: vi.fn().mockReturnValue({
|
||||||
|
getChatRecordingService: vi.fn().mockReturnValue({
|
||||||
|
getConversation: vi.fn().mockReturnValue({ messages: [] }),
|
||||||
|
getConversationFilePath: vi
|
||||||
|
.fn()
|
||||||
|
.mockReturnValue('/tmp/transcript.json'),
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
getContentGenerator: vi.fn(),
|
||||||
|
getActiveModel: vi.fn().mockReturnValue('gemini-pro'),
|
||||||
|
getWorkingDir: vi.fn().mockReturnValue('/work'),
|
||||||
|
} as unknown as Config;
|
||||||
|
|
||||||
|
hookRunner = new HookRunner(mockConfig);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should execute session-learnings builtin hook on SessionEnd', async () => {
|
||||||
|
const hookConfig: BuiltinHookConfig = {
|
||||||
|
type: HookType.Builtin,
|
||||||
|
builtin_id: 'session-learnings',
|
||||||
|
name: 'test-learnings',
|
||||||
|
};
|
||||||
|
|
||||||
|
const input: SessionEndInput = {
|
||||||
|
session_id: 'test-session',
|
||||||
|
transcript_path: '/tmp/transcript.json',
|
||||||
|
cwd: '/work',
|
||||||
|
hook_event_name: HookEventName.SessionEnd,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
reason: SessionEndReason.Exit,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Spy on the service
|
||||||
|
const serviceSpy = vi
|
||||||
|
.spyOn(
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
(hookRunner as any).sessionLearningsService,
|
||||||
|
'generateAndSaveLearnings',
|
||||||
|
)
|
||||||
|
.mockResolvedValue(undefined);
|
||||||
|
|
||||||
|
const result = await hookRunner.executeHook(
|
||||||
|
hookConfig,
|
||||||
|
HookEventName.SessionEnd,
|
||||||
|
input,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
expect(serviceSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not execute session-learnings if reason is not exit/logout', async () => {
|
||||||
|
const hookConfig: BuiltinHookConfig = {
|
||||||
|
type: HookType.Builtin,
|
||||||
|
builtin_id: 'session-learnings',
|
||||||
|
};
|
||||||
|
|
||||||
|
const input: SessionEndInput = {
|
||||||
|
session_id: 'test-session',
|
||||||
|
transcript_path: '/tmp/transcript.json',
|
||||||
|
cwd: '/work',
|
||||||
|
hook_event_name: HookEventName.SessionEnd,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
reason: SessionEndReason.Clear,
|
||||||
|
};
|
||||||
|
|
||||||
|
const serviceSpy = vi.spyOn(
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
(hookRunner as any).sessionLearningsService,
|
||||||
|
'generateAndSaveLearnings',
|
||||||
|
);
|
||||||
|
|
||||||
|
const result = await hookRunner.executeHook(
|
||||||
|
hookConfig,
|
||||||
|
HookEventName.SessionEnd,
|
||||||
|
input,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
expect(serviceSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -6,7 +6,7 @@
|
|||||||
|
|
||||||
import { spawn } from 'node:child_process';
|
import { spawn } from 'node:child_process';
|
||||||
import type { HookConfig } from './types.js';
|
import type { HookConfig } 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 { Config } from '../config/config.js';
|
||||||
import type {
|
import type {
|
||||||
HookInput,
|
HookInput,
|
||||||
@@ -16,7 +16,9 @@ import type {
|
|||||||
BeforeModelInput,
|
BeforeModelInput,
|
||||||
BeforeModelOutput,
|
BeforeModelOutput,
|
||||||
BeforeToolInput,
|
BeforeToolInput,
|
||||||
|
SessionEndInput,
|
||||||
} from './types.js';
|
} from './types.js';
|
||||||
|
import { SessionEndReason } from './types.js';
|
||||||
import type { LLMRequest } from './hookTranslator.js';
|
import type { LLMRequest } from './hookTranslator.js';
|
||||||
import { debugLogger } from '../utils/debugLogger.js';
|
import { debugLogger } from '../utils/debugLogger.js';
|
||||||
import { sanitizeEnvironment } from '../services/environmentSanitization.js';
|
import { sanitizeEnvironment } from '../services/environmentSanitization.js';
|
||||||
@@ -25,6 +27,7 @@ import {
|
|||||||
getShellConfiguration,
|
getShellConfiguration,
|
||||||
type ShellType,
|
type ShellType,
|
||||||
} from '../utils/shell-utils.js';
|
} from '../utils/shell-utils.js';
|
||||||
|
import { SessionLearningsService } from '../services/sessionLearningsService.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Default timeout for hook execution (60 seconds)
|
* Default timeout for hook execution (60 seconds)
|
||||||
@@ -43,9 +46,11 @@ const EXIT_CODE_NON_BLOCKING_ERROR = 1;
|
|||||||
*/
|
*/
|
||||||
export class HookRunner {
|
export class HookRunner {
|
||||||
private readonly config: Config;
|
private readonly config: Config;
|
||||||
|
private readonly sessionLearningsService: SessionLearningsService;
|
||||||
|
|
||||||
constructor(config: Config) {
|
constructor(config: Config) {
|
||||||
this.config = config;
|
this.config = config;
|
||||||
|
this.sessionLearningsService = new SessionLearningsService(config);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -76,12 +81,25 @@ export class HookRunner {
|
|||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return await this.executeCommandHook(
|
if (hookConfig.type === HookType.Command) {
|
||||||
hookConfig,
|
return await this.executeCommandHook(
|
||||||
eventName,
|
hookConfig,
|
||||||
input,
|
eventName,
|
||||||
startTime,
|
input,
|
||||||
);
|
startTime,
|
||||||
|
);
|
||||||
|
} else if (hookConfig.type === HookType.Builtin) {
|
||||||
|
return await this.executeBuiltinHook(
|
||||||
|
hookConfig,
|
||||||
|
eventName,
|
||||||
|
input,
|
||||||
|
startTime,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
throw new Error(
|
||||||
|
`Unsupported hook type: ${(hookConfig as HookConfig).type}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const duration = Date.now() - startTime;
|
const duration = Date.now() - startTime;
|
||||||
const hookId = hookConfig.name || hookConfig.command || 'unknown';
|
const hookId = hookConfig.name || hookConfig.command || 'unknown';
|
||||||
@@ -231,6 +249,52 @@ export class HookRunner {
|
|||||||
return modifiedInput;
|
return modifiedInput;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Execute a builtin hook
|
||||||
|
*/
|
||||||
|
private async executeBuiltinHook(
|
||||||
|
hookConfig: HookConfig,
|
||||||
|
eventName: HookEventName,
|
||||||
|
input: HookInput,
|
||||||
|
startTime: number,
|
||||||
|
): Promise<HookExecutionResult> {
|
||||||
|
if (hookConfig.type !== HookType.Builtin) {
|
||||||
|
throw new Error('Expected builtin hook configuration');
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
if (hookConfig.builtin_id === 'session-learnings') {
|
||||||
|
if (eventName === HookEventName.SessionEnd) {
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
||||||
|
const sessionEndInput = input as SessionEndInput;
|
||||||
|
if (
|
||||||
|
sessionEndInput.reason === SessionEndReason.Exit ||
|
||||||
|
sessionEndInput.reason === SessionEndReason.Logout
|
||||||
|
) {
|
||||||
|
await this.sessionLearningsService.generateAndSaveLearnings();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
hookConfig,
|
||||||
|
eventName,
|
||||||
|
success: true,
|
||||||
|
duration: Date.now() - startTime,
|
||||||
|
exitCode: EXIT_CODE_SUCCESS,
|
||||||
|
output: {},
|
||||||
|
};
|
||||||
|
} catch (error) {
|
||||||
|
return {
|
||||||
|
hookConfig,
|
||||||
|
eventName,
|
||||||
|
success: false,
|
||||||
|
error: error instanceof Error ? error : new Error(String(error)),
|
||||||
|
duration: Date.now() - startTime,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Execute a command hook
|
* Execute a command hook
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -33,7 +33,6 @@ import type {
|
|||||||
ToolListUnion,
|
ToolListUnion,
|
||||||
} from '@google/genai';
|
} from '@google/genai';
|
||||||
import type { ToolCallConfirmationDetails } from '../tools/tools.js';
|
import type { ToolCallConfirmationDetails } from '../tools/tools.js';
|
||||||
import { SessionLearningsService } from '../services/sessionLearningsService.js';
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Main hook system that coordinates all hook-related functionality
|
* Main hook system that coordinates all hook-related functionality
|
||||||
@@ -152,7 +151,6 @@ export class HookSystem {
|
|||||||
private readonly hookAggregator: HookAggregator;
|
private readonly hookAggregator: HookAggregator;
|
||||||
private readonly hookPlanner: HookPlanner;
|
private readonly hookPlanner: HookPlanner;
|
||||||
private readonly hookEventHandler: HookEventHandler;
|
private readonly hookEventHandler: HookEventHandler;
|
||||||
private readonly sessionLearningsService: SessionLearningsService;
|
|
||||||
|
|
||||||
constructor(config: Config) {
|
constructor(config: Config) {
|
||||||
// Initialize components
|
// Initialize components
|
||||||
@@ -166,7 +164,6 @@ export class HookSystem {
|
|||||||
this.hookRunner,
|
this.hookRunner,
|
||||||
this.hookAggregator,
|
this.hookAggregator,
|
||||||
);
|
);
|
||||||
this.sessionLearningsService = new SessionLearningsService(config);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -218,14 +215,7 @@ export class HookSystem {
|
|||||||
async fireSessionEndEvent(
|
async fireSessionEndEvent(
|
||||||
reason: SessionEndReason,
|
reason: SessionEndReason,
|
||||||
): Promise<AggregatedHookResult | undefined> {
|
): Promise<AggregatedHookResult | undefined> {
|
||||||
const result = await this.hookEventHandler.fireSessionEndEvent(reason);
|
return this.hookEventHandler.fireSessionEndEvent(reason);
|
||||||
|
|
||||||
// Built-in system hook for session learnings
|
|
||||||
if (reason === 'exit' || reason === 'logout') {
|
|
||||||
await this.sessionLearningsService.generateAndSaveLearnings();
|
|
||||||
}
|
|
||||||
|
|
||||||
return result;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async firePreCompressEvent(
|
async firePreCompressEvent(
|
||||||
|
|||||||
@@ -62,7 +62,16 @@ export interface CommandHookConfig {
|
|||||||
env?: Record<string, string>;
|
env?: Record<string, string>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type HookConfig = CommandHookConfig;
|
export interface BuiltinHookConfig {
|
||||||
|
type: HookType.Builtin;
|
||||||
|
builtin_id: string;
|
||||||
|
name?: string;
|
||||||
|
description?: string;
|
||||||
|
timeout?: number;
|
||||||
|
source?: ConfigSource;
|
||||||
|
}
|
||||||
|
|
||||||
|
export type HookConfig = CommandHookConfig | BuiltinHookConfig;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Hook definition with matcher
|
* Hook definition with matcher
|
||||||
@@ -78,6 +87,7 @@ export interface HookDefinition {
|
|||||||
*/
|
*/
|
||||||
export enum HookType {
|
export enum HookType {
|
||||||
Command = 'command',
|
Command = 'command',
|
||||||
|
Builtin = 'builtin',
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -85,8 +95,9 @@ export enum HookType {
|
|||||||
*/
|
*/
|
||||||
export function getHookKey(hook: HookConfig): string {
|
export function getHookKey(hook: HookConfig): string {
|
||||||
const name = hook.name || '';
|
const name = hook.name || '';
|
||||||
const command = hook.command || '';
|
const identifier =
|
||||||
return `${name}:${command}`;
|
hook.type === HookType.Command ? hook.command : hook.builtin_id;
|
||||||
|
return `${name}:${identifier}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user