diff --git a/packages/core/src/hooks/hookPlanner.test.ts b/packages/core/src/hooks/hookPlanner.test.ts new file mode 100644 index 0000000000..c408266ecb --- /dev/null +++ b/packages/core/src/hooks/hookPlanner.test.ts @@ -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'); + }); + }); +}); diff --git a/packages/core/src/hooks/hookPlanner.ts b/packages/core/src/hooks/hookPlanner.ts new file mode 100644 index 0000000000..4a0f00a8e1 --- /dev/null +++ b/packages/core/src/hooks/hookPlanner.ts @@ -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(); + 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; +} diff --git a/packages/core/src/hooks/hookRegistry.test.ts b/packages/core/src/hooks/hookRegistry.test.ts new file mode 100644 index 0000000000..5c6906b838 --- /dev/null +++ b/packages/core/src/hooks/hookRegistry.test.ts @@ -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' }), + ); + }); + }); +}); diff --git a/packages/core/src/hooks/hookRegistry.ts b/packages/core/src/hooks/hookRegistry.ts new file mode 100644 index 0000000000..82fda32975 --- /dev/null +++ b/packages/core/src/hooks/hookRegistry.ts @@ -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 { + 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; + } + } +}