mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-12 07:01:09 -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([]),
|
||||
isTrustedFolder: vi.fn().mockReturnValue(true),
|
||||
getProjectRoot: vi.fn().mockReturnValue('/project'),
|
||||
isSessionLearningsEnabled: vi.fn().mockReturnValue(false),
|
||||
} as unknown as Config;
|
||||
|
||||
hookRegistry = new HookRegistry(mockConfig);
|
||||
@@ -279,6 +280,21 @@ describe('HookRegistry', () => {
|
||||
hookRegistry.getHooksForEvent(HookEventName.BeforeTool),
|
||||
).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', () => {
|
||||
|
||||
@@ -6,7 +6,12 @@
|
||||
|
||||
import type { Config } from '../config/config.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 { TrustedHooksManager } from './trustedHooks.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.registerBuiltinHooks();
|
||||
|
||||
// Get hooks from the main config (this comes from the merged settings)
|
||||
const configHooks = this.config.getHooks();
|
||||
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
|
||||
*/
|
||||
|
||||
109
packages/core/src/hooks/hookRunner.builtin.test.ts
Normal file
109
packages/core/src/hooks/hookRunner.builtin.test.ts
Normal file
@@ -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 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 {
|
||||
HookInput,
|
||||
@@ -16,7 +16,9 @@ import type {
|
||||
BeforeModelInput,
|
||||
BeforeModelOutput,
|
||||
BeforeToolInput,
|
||||
SessionEndInput,
|
||||
} from './types.js';
|
||||
import { SessionEndReason } from './types.js';
|
||||
import type { LLMRequest } from './hookTranslator.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { sanitizeEnvironment } from '../services/environmentSanitization.js';
|
||||
@@ -25,6 +27,7 @@ import {
|
||||
getShellConfiguration,
|
||||
type ShellType,
|
||||
} from '../utils/shell-utils.js';
|
||||
import { SessionLearningsService } from '../services/sessionLearningsService.js';
|
||||
|
||||
/**
|
||||
* Default timeout for hook execution (60 seconds)
|
||||
@@ -43,9 +46,11 @@ const EXIT_CODE_NON_BLOCKING_ERROR = 1;
|
||||
*/
|
||||
export class HookRunner {
|
||||
private readonly config: Config;
|
||||
private readonly sessionLearningsService: SessionLearningsService;
|
||||
|
||||
constructor(config: Config) {
|
||||
this.config = config;
|
||||
this.sessionLearningsService = new SessionLearningsService(config);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -76,12 +81,25 @@ export class HookRunner {
|
||||
}
|
||||
|
||||
try {
|
||||
return await this.executeCommandHook(
|
||||
hookConfig,
|
||||
eventName,
|
||||
input,
|
||||
startTime,
|
||||
);
|
||||
if (hookConfig.type === HookType.Command) {
|
||||
return await this.executeCommandHook(
|
||||
hookConfig,
|
||||
eventName,
|
||||
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) {
|
||||
const duration = Date.now() - startTime;
|
||||
const hookId = hookConfig.name || hookConfig.command || 'unknown';
|
||||
@@ -231,6 +249,52 @@ export class HookRunner {
|
||||
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
|
||||
*/
|
||||
|
||||
@@ -33,7 +33,6 @@ import type {
|
||||
ToolListUnion,
|
||||
} from '@google/genai';
|
||||
import type { ToolCallConfirmationDetails } from '../tools/tools.js';
|
||||
import { SessionLearningsService } from '../services/sessionLearningsService.js';
|
||||
|
||||
/**
|
||||
* Main hook system that coordinates all hook-related functionality
|
||||
@@ -152,7 +151,6 @@ export class HookSystem {
|
||||
private readonly hookAggregator: HookAggregator;
|
||||
private readonly hookPlanner: HookPlanner;
|
||||
private readonly hookEventHandler: HookEventHandler;
|
||||
private readonly sessionLearningsService: SessionLearningsService;
|
||||
|
||||
constructor(config: Config) {
|
||||
// Initialize components
|
||||
@@ -166,7 +164,6 @@ export class HookSystem {
|
||||
this.hookRunner,
|
||||
this.hookAggregator,
|
||||
);
|
||||
this.sessionLearningsService = new SessionLearningsService(config);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -218,14 +215,7 @@ export class HookSystem {
|
||||
async fireSessionEndEvent(
|
||||
reason: SessionEndReason,
|
||||
): Promise<AggregatedHookResult | undefined> {
|
||||
const result = await this.hookEventHandler.fireSessionEndEvent(reason);
|
||||
|
||||
// Built-in system hook for session learnings
|
||||
if (reason === 'exit' || reason === 'logout') {
|
||||
await this.sessionLearningsService.generateAndSaveLearnings();
|
||||
}
|
||||
|
||||
return result;
|
||||
return this.hookEventHandler.fireSessionEndEvent(reason);
|
||||
}
|
||||
|
||||
async firePreCompressEvent(
|
||||
|
||||
@@ -62,7 +62,16 @@ export interface CommandHookConfig {
|
||||
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
|
||||
@@ -78,6 +87,7 @@ export interface HookDefinition {
|
||||
*/
|
||||
export enum HookType {
|
||||
Command = 'command',
|
||||
Builtin = 'builtin',
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -85,8 +95,9 @@ export enum HookType {
|
||||
*/
|
||||
export function getHookKey(hook: HookConfig): string {
|
||||
const name = hook.name || '';
|
||||
const command = hook.command || '';
|
||||
return `${name}:${command}`;
|
||||
const identifier =
|
||||
hook.type === HookType.Command ? hook.command : hook.builtin_id;
|
||||
return `${name}:${identifier}`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user