mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-13 23:51:16 -07:00
feat(hooks): Hook Execution Planning and Matching (#9090)
This commit is contained in:
324
packages/core/src/hooks/hookPlanner.test.ts
Normal file
324
packages/core/src/hooks/hookPlanner.test.ts
Normal file
@@ -0,0 +1,324 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { HookPlanner } from './hookPlanner.js';
|
||||
import type { HookRegistry, HookRegistryEntry } from './hookRegistry.js';
|
||||
import { HookEventName, HookType } from './types.js';
|
||||
import { ConfigSource } from './hookRegistry.js';
|
||||
|
||||
// Mock debugLogger using vi.hoisted
|
||||
const mockDebugLogger = vi.hoisted(() => ({
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/debugLogger.js', () => ({
|
||||
debugLogger: mockDebugLogger,
|
||||
}));
|
||||
|
||||
describe('HookPlanner', () => {
|
||||
let hookPlanner: HookPlanner;
|
||||
let mockHookRegistry: HookRegistry;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
|
||||
mockHookRegistry = {
|
||||
getHooksForEvent: vi.fn(),
|
||||
} as unknown as HookRegistry;
|
||||
|
||||
hookPlanner = new HookPlanner(mockHookRegistry);
|
||||
});
|
||||
|
||||
describe('createExecutionPlan', () => {
|
||||
it('should return empty plan when no hooks registered', () => {
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue([]);
|
||||
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool);
|
||||
|
||||
expect(plan).toBeNull();
|
||||
});
|
||||
|
||||
it('should create plan for hooks without matchers', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './hook1.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: {
|
||||
type: HookType.Command,
|
||||
command: './test-hook.sh',
|
||||
},
|
||||
source: ConfigSource.User,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool);
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(2);
|
||||
expect(plan!.hookConfigs[0].command).toBe('./hook1.sh');
|
||||
expect(plan!.hookConfigs[1].command).toBe('./test-hook.sh');
|
||||
});
|
||||
|
||||
it('should filter hooks by tool name matcher', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './edit_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: 'EditTool',
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: { type: HookType.Command, command: './general_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
// Test with EditTool context
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool, {
|
||||
toolName: 'EditTool',
|
||||
});
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(2); // Both should match (one specific, one general)
|
||||
});
|
||||
|
||||
it('should filter hooks by regex matcher', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './edit_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: 'Edit|Write',
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: { type: HookType.Command, command: './read_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: 'ReadTool',
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
// Test with EditTool - should match first hook
|
||||
const editPlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.BeforeTool,
|
||||
{ toolName: 'EditTool' },
|
||||
);
|
||||
expect(editPlan).not.toBeNull();
|
||||
expect(editPlan!.hookConfigs).toHaveLength(1);
|
||||
expect(editPlan!.hookConfigs[0].command).toBe('./edit_hook.sh');
|
||||
|
||||
// Test with WriteTool - should match first hook
|
||||
const writePlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.BeforeTool,
|
||||
{ toolName: 'WriteTool' },
|
||||
);
|
||||
expect(writePlan).not.toBeNull();
|
||||
expect(writePlan!.hookConfigs).toHaveLength(1);
|
||||
expect(writePlan!.hookConfigs[0].command).toBe('./edit_hook.sh');
|
||||
|
||||
// Test with ReadTool - should match second hook
|
||||
const readPlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.BeforeTool,
|
||||
{ toolName: 'ReadTool' },
|
||||
);
|
||||
expect(readPlan).not.toBeNull();
|
||||
expect(readPlan!.hookConfigs).toHaveLength(1);
|
||||
expect(readPlan!.hookConfigs[0].command).toBe('./read_hook.sh');
|
||||
|
||||
// Test with unmatched tool - should match no hooks
|
||||
const otherPlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.BeforeTool,
|
||||
{ toolName: 'OtherTool' },
|
||||
);
|
||||
expect(otherPlan).toBeNull();
|
||||
});
|
||||
|
||||
it('should handle wildcard matcher', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './wildcard_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: '*',
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool, {
|
||||
toolName: 'AnyTool',
|
||||
});
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should handle empty string matcher', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: {
|
||||
type: HookType.Command,
|
||||
command: './empty_matcher_hook.sh',
|
||||
},
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: '',
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool, {
|
||||
toolName: 'AnyTool',
|
||||
});
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should handle invalid regex matcher gracefully', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: {
|
||||
type: HookType.Command,
|
||||
command: './invalid_regex_hook.sh',
|
||||
},
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
matcher: '[invalid-regex',
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
// Should match when toolName exactly equals the invalid regex pattern
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool, {
|
||||
toolName: '[invalid-regex',
|
||||
});
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(1); // Should fall back to exact match
|
||||
|
||||
// Should not match when toolName doesn't exactly equal the pattern
|
||||
const planNoMatch = hookPlanner.createExecutionPlan(
|
||||
HookEventName.BeforeTool,
|
||||
{
|
||||
toolName: 'other-tool',
|
||||
},
|
||||
);
|
||||
|
||||
expect(planNoMatch).toBeNull();
|
||||
});
|
||||
|
||||
it('should deduplicate identical hooks', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './same_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: { type: HookType.Command, command: './same_hook.sh' },
|
||||
source: ConfigSource.User,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: {
|
||||
type: HookType.Command,
|
||||
command: './test-hook.sh',
|
||||
},
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: {
|
||||
type: HookType.Command,
|
||||
command: './test-hook.sh',
|
||||
},
|
||||
source: ConfigSource.User,
|
||||
eventName: HookEventName.BeforeTool,
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool);
|
||||
|
||||
expect(plan).not.toBeNull();
|
||||
expect(plan!.hookConfigs).toHaveLength(2); // Should be deduplicated to 2 unique hooks
|
||||
expect(mockDebugLogger.debug).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Deduplicated hook'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should match trigger for session events', () => {
|
||||
const mockEntries: HookRegistryEntry[] = [
|
||||
{
|
||||
config: { type: HookType.Command, command: './startup_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.SessionStart,
|
||||
matcher: 'startup',
|
||||
enabled: true,
|
||||
},
|
||||
{
|
||||
config: { type: HookType.Command, command: './resume_hook.sh' },
|
||||
source: ConfigSource.Project,
|
||||
eventName: HookEventName.SessionStart,
|
||||
matcher: 'resume',
|
||||
enabled: true,
|
||||
},
|
||||
];
|
||||
|
||||
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
|
||||
|
||||
// Test startup trigger
|
||||
const startupPlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.SessionStart,
|
||||
{ trigger: 'startup' },
|
||||
);
|
||||
expect(startupPlan).not.toBeNull();
|
||||
expect(startupPlan!.hookConfigs).toHaveLength(1);
|
||||
expect(startupPlan!.hookConfigs[0].command).toBe('./startup_hook.sh');
|
||||
|
||||
// Test resume trigger
|
||||
const resumePlan = hookPlanner.createExecutionPlan(
|
||||
HookEventName.SessionStart,
|
||||
{ trigger: 'resume' },
|
||||
);
|
||||
expect(resumePlan).not.toBeNull();
|
||||
expect(resumePlan!.hookConfigs).toHaveLength(1);
|
||||
expect(resumePlan!.hookConfigs[0].command).toBe('./resume_hook.sh');
|
||||
});
|
||||
});
|
||||
});
|
||||
154
packages/core/src/hooks/hookPlanner.ts
Normal file
154
packages/core/src/hooks/hookPlanner.ts
Normal file
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import type { HookRegistry, HookRegistryEntry } from './hookRegistry.js';
|
||||
import type { HookExecutionPlan } from './types.js';
|
||||
import type { HookEventName } from './types.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
|
||||
/**
|
||||
* Hook planner that selects matching hooks and creates execution plans
|
||||
*/
|
||||
export class HookPlanner {
|
||||
private readonly hookRegistry: HookRegistry;
|
||||
|
||||
constructor(hookRegistry: HookRegistry) {
|
||||
this.hookRegistry = hookRegistry;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create execution plan for a hook event
|
||||
*/
|
||||
createExecutionPlan(
|
||||
eventName: HookEventName,
|
||||
context?: HookEventContext,
|
||||
): HookExecutionPlan | null {
|
||||
const hookEntries = this.hookRegistry.getHooksForEvent(eventName);
|
||||
|
||||
if (hookEntries.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Filter hooks by matcher
|
||||
const matchingEntries = hookEntries.filter((entry) =>
|
||||
this.matchesContext(entry, context),
|
||||
);
|
||||
|
||||
if (matchingEntries.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Deduplicate identical hooks
|
||||
const deduplicatedEntries = this.deduplicateHooks(matchingEntries);
|
||||
|
||||
// Extract hook configs
|
||||
const hookConfigs = deduplicatedEntries.map((entry) => entry.config);
|
||||
|
||||
// Determine execution strategy - if ANY hook definition has sequential=true, run all sequentially
|
||||
const sequential = deduplicatedEntries.some(
|
||||
(entry) => entry.sequential === true,
|
||||
);
|
||||
|
||||
const plan: HookExecutionPlan = {
|
||||
eventName,
|
||||
hookConfigs,
|
||||
sequential,
|
||||
};
|
||||
|
||||
debugLogger.debug(
|
||||
`Created execution plan for ${eventName}: ${hookConfigs.length} hook(s) to execute ${sequential ? 'sequentially' : 'in parallel'}`,
|
||||
);
|
||||
|
||||
return plan;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a hook entry matches the given context
|
||||
*/
|
||||
private matchesContext(
|
||||
entry: HookRegistryEntry,
|
||||
context?: HookEventContext,
|
||||
): boolean {
|
||||
if (!entry.matcher || !context) {
|
||||
return true; // No matcher means match all
|
||||
}
|
||||
|
||||
const matcher = entry.matcher.trim();
|
||||
|
||||
if (matcher === '' || matcher === '*') {
|
||||
return true; // Empty string or wildcard matches all
|
||||
}
|
||||
|
||||
// For tool events, match against tool name
|
||||
if (context.toolName) {
|
||||
return this.matchesToolName(matcher, context.toolName);
|
||||
}
|
||||
|
||||
// For other events, match against trigger/source
|
||||
if (context.trigger) {
|
||||
return this.matchesTrigger(matcher, context.trigger);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Match tool name against matcher pattern
|
||||
*/
|
||||
private matchesToolName(matcher: string, toolName: string): boolean {
|
||||
try {
|
||||
// Attempt to treat the matcher as a regular expression.
|
||||
const regex = new RegExp(matcher);
|
||||
return regex.test(toolName);
|
||||
} catch {
|
||||
// If it's not a valid regex, treat it as a literal string for an exact match.
|
||||
return matcher === toolName;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Match trigger/source against matcher pattern
|
||||
*/
|
||||
private matchesTrigger(matcher: string, trigger: string): boolean {
|
||||
return matcher === trigger;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deduplicate identical hook configurations
|
||||
*/
|
||||
private deduplicateHooks(entries: HookRegistryEntry[]): HookRegistryEntry[] {
|
||||
const seen = new Set<string>();
|
||||
const deduplicated: HookRegistryEntry[] = [];
|
||||
|
||||
for (const entry of entries) {
|
||||
const key = this.getHookKey(entry);
|
||||
|
||||
if (!seen.has(key)) {
|
||||
seen.add(key);
|
||||
deduplicated.push(entry);
|
||||
} else {
|
||||
debugLogger.debug(`Deduplicated hook: ${key}`);
|
||||
}
|
||||
}
|
||||
|
||||
return deduplicated;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a unique key for a hook entry
|
||||
*/
|
||||
private getHookKey(entry: HookRegistryEntry): string {
|
||||
return `command:${entry.config.command}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Context information for hook event matching
|
||||
*/
|
||||
export interface HookEventContext {
|
||||
toolName?: string;
|
||||
trigger?: string;
|
||||
}
|
||||
504
packages/core/src/hooks/hookRegistry.test.ts
Normal file
504
packages/core/src/hooks/hookRegistry.test.ts
Normal file
@@ -0,0 +1,504 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'node:fs';
|
||||
import {
|
||||
HookRegistry,
|
||||
ConfigSource,
|
||||
HookRegistryNotInitializedError,
|
||||
} from './hookRegistry.js';
|
||||
import type { Storage } from '../config/storage.js';
|
||||
import { HookEventName, HookType } from './types.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { HookDefinition } from './types.js';
|
||||
|
||||
// Mock fs
|
||||
vi.mock('fs', () => ({
|
||||
existsSync: vi.fn(),
|
||||
readFileSync: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock debugLogger using vi.hoisted
|
||||
const mockDebugLogger = vi.hoisted(() => ({
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/debugLogger.js', () => ({
|
||||
debugLogger: mockDebugLogger,
|
||||
}));
|
||||
|
||||
describe('HookRegistry', () => {
|
||||
let hookRegistry: HookRegistry;
|
||||
let mockConfig: Config;
|
||||
let mockStorage: Storage;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
|
||||
mockStorage = {
|
||||
getGeminiDir: vi.fn().mockReturnValue('/project/.gemini'),
|
||||
} as unknown as Storage;
|
||||
|
||||
mockConfig = {
|
||||
storage: mockStorage,
|
||||
getExtensions: vi.fn().mockReturnValue([]),
|
||||
getHooks: vi.fn().mockReturnValue({}),
|
||||
} as unknown as Config;
|
||||
|
||||
hookRegistry = new HookRegistry(mockConfig);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('initialize', () => {
|
||||
it('should initialize successfully with no hooks', async () => {
|
||||
vi.mocked(fs.existsSync).mockReturnValue(false);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.log).toHaveBeenCalledWith(
|
||||
'Hook registry initialized with 0 hook entries',
|
||||
);
|
||||
});
|
||||
|
||||
it('should load hooks from project configuration', async () => {
|
||||
const mockHooksConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
matcher: 'EditTool',
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/check_style.sh',
|
||||
timeout: 60,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return the hooks configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mockHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
const hooks = hookRegistry.getAllHooks();
|
||||
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].matcher).toBe('EditTool');
|
||||
expect(hooks[0].source).toBe(ConfigSource.Project);
|
||||
});
|
||||
|
||||
it('should load plugin hooks', async () => {
|
||||
const mockHooksConfig = {
|
||||
AfterTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/after-tool.sh',
|
||||
timeout: 30,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return the hooks configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mockHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
const hooks = hookRegistry.getAllHooks();
|
||||
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');
|
||||
});
|
||||
|
||||
it('should handle invalid configuration gracefully', async () => {
|
||||
const invalidHooksConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'invalid-type', // Invalid hook type
|
||||
command: './hooks/test.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return invalid configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
invalidHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should validate hook configurations', async () => {
|
||||
const mockHooksConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'invalid',
|
||||
command: './hooks/test.sh',
|
||||
},
|
||||
{
|
||||
type: 'command',
|
||||
// Missing command field
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return invalid configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mockHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalled(); // At least some warnings should be logged
|
||||
});
|
||||
});
|
||||
|
||||
describe('getHooksForEvent', () => {
|
||||
beforeEach(async () => {
|
||||
const mockHooksConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
matcher: 'EditTool',
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/edit_check.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/general_check.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
AfterTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/after-tool.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return the hooks configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mockHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
});
|
||||
|
||||
it('should return hooks for specific event', () => {
|
||||
const beforeToolHooks = hookRegistry.getHooksForEvent(
|
||||
HookEventName.BeforeTool,
|
||||
);
|
||||
expect(beforeToolHooks).toHaveLength(2);
|
||||
|
||||
const afterToolHooks = hookRegistry.getHooksForEvent(
|
||||
HookEventName.AfterTool,
|
||||
);
|
||||
expect(afterToolHooks).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should return empty array for events with no hooks', () => {
|
||||
const notificationHooks = hookRegistry.getHooksForEvent(
|
||||
HookEventName.Notification,
|
||||
);
|
||||
expect(notificationHooks).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should throw error if not initialized', () => {
|
||||
const uninitializedRegistry = new HookRegistry(mockConfig);
|
||||
|
||||
expect(() => {
|
||||
uninitializedRegistry.getHooksForEvent(HookEventName.BeforeTool);
|
||||
}).toThrow(HookRegistryNotInitializedError);
|
||||
});
|
||||
});
|
||||
|
||||
describe('setHookEnabled', () => {
|
||||
beforeEach(async () => {
|
||||
const mockHooksConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './hooks/test.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
// Update mock to return the hooks configuration
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mockHooksConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
});
|
||||
|
||||
it('should enable and disable hooks', () => {
|
||||
const hookName = './hooks/test.sh';
|
||||
|
||||
// Initially enabled
|
||||
let hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
|
||||
expect(hooks).toHaveLength(1);
|
||||
|
||||
// Disable
|
||||
hookRegistry.setHookEnabled(hookName, false);
|
||||
hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
|
||||
expect(hooks).toHaveLength(0);
|
||||
|
||||
// Re-enable
|
||||
hookRegistry.setHookEnabled(hookName, true);
|
||||
hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
|
||||
expect(hooks).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should warn when hook not found', () => {
|
||||
hookRegistry.setHookEnabled('non-existent-hook', false);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
'No hooks found matching "non-existent-hook"',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('malformed configuration handling', () => {
|
||||
it('should handle non-array definitions gracefully', async () => {
|
||||
const malformedConfig = {
|
||||
BeforeTool: 'not-an-array', // Should be an array of HookDefinition
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('is not an array'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle object instead of array for definitions', async () => {
|
||||
const malformedConfig = {
|
||||
AfterTool: { hooks: [] }, // Should be an array, not a single object
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('is not an array'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle null definition gracefully', async () => {
|
||||
const malformedConfig = {
|
||||
BeforeTool: [null], // Invalid: null definition
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Discarding invalid hook definition'),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle definition without hooks array', async () => {
|
||||
const malformedConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
matcher: 'EditTool',
|
||||
// Missing hooks array
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Discarding invalid hook definition'),
|
||||
expect.objectContaining({ matcher: 'EditTool' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle non-array hooks property', async () => {
|
||||
const malformedConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
matcher: 'EditTool',
|
||||
hooks: 'not-an-array', // Should be an array
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Discarding invalid hook definition'),
|
||||
expect.objectContaining({ hooks: 'not-an-array', matcher: 'EditTool' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle non-object hookConfig in hooks array', async () => {
|
||||
const malformedConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
hooks: [
|
||||
'not-an-object', // Should be an object
|
||||
42, // Should be an object
|
||||
null, // Should be an object
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
malformedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
expect(hookRegistry.getAllHooks()).toHaveLength(0);
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledTimes(3); // One warning for each invalid hookConfig
|
||||
});
|
||||
|
||||
it('should handle mixed valid and invalid hook configurations', async () => {
|
||||
const mixedConfig = {
|
||||
BeforeTool: [
|
||||
{
|
||||
hooks: [
|
||||
{
|
||||
type: 'command',
|
||||
command: './valid-hook.sh',
|
||||
},
|
||||
'invalid-string',
|
||||
{
|
||||
type: 'invalid-type',
|
||||
command: './invalid-type.sh',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
vi.mocked(mockConfig.getHooks).mockReturnValue(
|
||||
mixedConfig as unknown as {
|
||||
[K in HookEventName]?: HookDefinition[];
|
||||
},
|
||||
);
|
||||
|
||||
await hookRegistry.initialize();
|
||||
|
||||
// Should only load the valid hook
|
||||
const hooks = hookRegistry.getAllHooks();
|
||||
expect(hooks).toHaveLength(1);
|
||||
expect(hooks[0].config.command).toBe('./valid-hook.sh');
|
||||
|
||||
// Verify the warnings for invalid configurations
|
||||
// 1st warning: non-object hookConfig ('invalid-string')
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Discarding invalid hook configuration'),
|
||||
'invalid-string',
|
||||
);
|
||||
// 2nd warning: validateHookConfig logs invalid type
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Invalid hook BeforeTool from project type'),
|
||||
);
|
||||
// 3rd warning: processHookDefinition logs the failed hookConfig
|
||||
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Discarding invalid hook configuration'),
|
||||
expect.objectContaining({ type: 'invalid-type' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
273
packages/core/src/hooks/hookRegistry.ts
Normal file
273
packages/core/src/hooks/hookRegistry.ts
Normal file
@@ -0,0 +1,273 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { HookDefinition, HookConfig } from './types.js';
|
||||
import { HookEventName } from './types.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
|
||||
/**
|
||||
* Error thrown when attempting to use HookRegistry before initialization
|
||||
*/
|
||||
export class HookRegistryNotInitializedError extends Error {
|
||||
constructor(message = 'Hook registry not initialized') {
|
||||
super(message);
|
||||
this.name = 'HookRegistryNotInitializedError';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Configuration source levels in precedence order (highest to lowest)
|
||||
*/
|
||||
export enum ConfigSource {
|
||||
Project = 'project',
|
||||
User = 'user',
|
||||
System = 'system',
|
||||
Extensions = 'extensions',
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook registry entry with source information
|
||||
*/
|
||||
export interface HookRegistryEntry {
|
||||
config: HookConfig;
|
||||
source: ConfigSource;
|
||||
eventName: HookEventName;
|
||||
matcher?: string;
|
||||
sequential?: boolean;
|
||||
enabled: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook registry that loads and validates hook definitions from multiple sources
|
||||
*/
|
||||
export class HookRegistry {
|
||||
private readonly config: Config;
|
||||
private entries: HookRegistryEntry[] = [];
|
||||
private initialized = false;
|
||||
|
||||
constructor(config: Config) {
|
||||
this.config = config;
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the registry by processing hooks from config
|
||||
*/
|
||||
async initialize(): Promise<void> {
|
||||
if (this.initialized) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.entries = [];
|
||||
this.processHooksFromConfig();
|
||||
this.initialized = true;
|
||||
|
||||
debugLogger.log(
|
||||
`Hook registry initialized with ${this.entries.length} hook entries`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all hook entries for a specific event
|
||||
*/
|
||||
getHooksForEvent(eventName: HookEventName): HookRegistryEntry[] {
|
||||
if (!this.initialized) {
|
||||
throw new HookRegistryNotInitializedError();
|
||||
}
|
||||
|
||||
return this.entries
|
||||
.filter((entry) => entry.eventName === eventName && entry.enabled)
|
||||
.sort(
|
||||
(a, b) =>
|
||||
this.getSourcePriority(a.source) - this.getSourcePriority(b.source),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all registered hooks
|
||||
*/
|
||||
getAllHooks(): HookRegistryEntry[] {
|
||||
if (!this.initialized) {
|
||||
throw new HookRegistryNotInitializedError();
|
||||
}
|
||||
|
||||
return [...this.entries];
|
||||
}
|
||||
|
||||
/**
|
||||
* Enable or disable a specific hook
|
||||
*/
|
||||
setHookEnabled(hookName: string, enabled: boolean): void {
|
||||
const updated = this.entries.filter((entry) => {
|
||||
const name = this.getHookName(entry);
|
||||
if (name === hookName) {
|
||||
entry.enabled = enabled;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
});
|
||||
|
||||
if (updated.length > 0) {
|
||||
debugLogger.log(
|
||||
`${enabled ? 'Enabled' : 'Disabled'} ${updated.length} hook(s) matching "${hookName}"`,
|
||||
);
|
||||
} else {
|
||||
debugLogger.warn(`No hooks found matching "${hookName}"`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get hook name for display purposes
|
||||
*/
|
||||
private getHookName(entry: HookRegistryEntry): string {
|
||||
return entry.config.command || 'unknown-command';
|
||||
}
|
||||
|
||||
/**
|
||||
* Process hooks from the config that was already loaded by the CLI
|
||||
*/
|
||||
private processHooksFromConfig(): void {
|
||||
// Get hooks from the main config (this comes from the merged settings)
|
||||
const configHooks = this.config.getHooks();
|
||||
if (configHooks) {
|
||||
this.processHooksConfiguration(configHooks, ConfigSource.Project);
|
||||
}
|
||||
|
||||
// Get hooks from extensions
|
||||
const extensions = this.config.getExtensions() || [];
|
||||
for (const extension of extensions) {
|
||||
if (extension.isActive && extension.hooks) {
|
||||
this.processHooksConfiguration(
|
||||
extension.hooks,
|
||||
ConfigSource.Extensions,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Process hooks configuration and add entries
|
||||
*/
|
||||
private processHooksConfiguration(
|
||||
hooksConfig: { [K in HookEventName]?: HookDefinition[] },
|
||||
source: ConfigSource,
|
||||
): void {
|
||||
for (const [eventName, definitions] of Object.entries(hooksConfig)) {
|
||||
if (!this.isValidEventName(eventName)) {
|
||||
debugLogger.warn(`Invalid hook event name: ${eventName}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const typedEventName = eventName as HookEventName;
|
||||
|
||||
if (!Array.isArray(definitions)) {
|
||||
debugLogger.warn(
|
||||
`Hook definitions for event "${eventName}" from source "${source}" is not an array. Skipping.`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const definition of definitions) {
|
||||
this.processHookDefinition(definition, typedEventName, source);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Process a single hook definition
|
||||
*/
|
||||
private processHookDefinition(
|
||||
definition: HookDefinition,
|
||||
eventName: HookEventName,
|
||||
source: ConfigSource,
|
||||
): void {
|
||||
if (
|
||||
!definition ||
|
||||
typeof definition !== 'object' ||
|
||||
!Array.isArray(definition.hooks)
|
||||
) {
|
||||
debugLogger.warn(
|
||||
`Discarding invalid hook definition for ${eventName} from ${source}:`,
|
||||
definition,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
for (const hookConfig of definition.hooks) {
|
||||
if (
|
||||
hookConfig &&
|
||||
typeof hookConfig === 'object' &&
|
||||
this.validateHookConfig(hookConfig, eventName, source)
|
||||
) {
|
||||
this.entries.push({
|
||||
config: hookConfig,
|
||||
source,
|
||||
eventName,
|
||||
matcher: definition.matcher,
|
||||
sequential: definition.sequential,
|
||||
enabled: true,
|
||||
});
|
||||
} else {
|
||||
// Invalid hooks are logged and discarded here, they won't reach HookRunner
|
||||
debugLogger.warn(
|
||||
`Discarding invalid hook configuration for ${eventName} from ${source}:`,
|
||||
hookConfig,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a hook configuration
|
||||
*/
|
||||
private validateHookConfig(
|
||||
config: HookConfig,
|
||||
eventName: HookEventName,
|
||||
source: ConfigSource,
|
||||
): boolean {
|
||||
if (!config.type || !['command', 'plugin'].includes(config.type)) {
|
||||
debugLogger.warn(
|
||||
`Invalid hook ${eventName} from ${source} type: ${config.type}`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (config.type === 'command' && !config.command) {
|
||||
debugLogger.warn(
|
||||
`Command hook ${eventName} from ${source} missing command field`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if an event name is valid
|
||||
*/
|
||||
private isValidEventName(eventName: string): eventName is HookEventName {
|
||||
const validEventNames = Object.values(HookEventName);
|
||||
return validEventNames.includes(eventName as HookEventName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get source priority (lower number = higher priority)
|
||||
*/
|
||||
private getSourcePriority(source: ConfigSource): number {
|
||||
switch (source) {
|
||||
case ConfigSource.Project:
|
||||
return 1;
|
||||
case ConfigSource.User:
|
||||
return 2;
|
||||
case ConfigSource.System:
|
||||
return 3;
|
||||
case ConfigSource.Extensions:
|
||||
return 4;
|
||||
default:
|
||||
return 999;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user