fix(core): surface warnings for invalid hook event names in configuration (#16788) (#16873)

packages/core/src/hooks/hookRegistry.test.ts

@@ -8,7 +8,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import * as fs from 'node:fs';
 import { HookRegistry } from './hookRegistry.js';
 import type { Storage } from '../config/storage.js';
-import { ConfigSource, HookEventName, HookType } from './types.js';
+import {
+  ConfigSource,
+  HookEventName,
+  HookType,
+  HOOKS_CONFIG_FIELDS,
+} from './types.js';
 import type { Config } from '../config/config.js';
 import type { HookDefinition } from './types.js';
 
@@ -645,6 +650,51 @@ describe('HookRegistry', () => {
         expect.objectContaining({ type: 'invalid-type' }),
       );
     });
+
+    it('should skip known config fields and warn on invalid event names', async () => {
+      const configWithExtras: Record<string, unknown> = {
+        InvalidEvent: [],
+        BeforeTool: [
+          {
+            hooks: [
+              {
+                type: 'command',
+                command: './test.sh',
+              },
+            ],
+          },
+        ],
+      };
+
+      // Add all known config fields dynamically
+      for (const field of HOOKS_CONFIG_FIELDS) {
+        configWithExtras[field] = field === 'disabled' ? [] : true;
+      }
+
+      vi.mocked(mockConfig.getHooks).mockReturnValue(
+        configWithExtras as unknown as {
+          [K in HookEventName]?: HookDefinition[];
+        },
+      );
+
+      await hookRegistry.initialize();
+
+      // Should only load the valid hook
+      expect(hookRegistry.getAllHooks()).toHaveLength(1);
+
+      // Should skip all known config fields without warnings
+      for (const field of HOOKS_CONFIG_FIELDS) {
+        expect(mockDebugLogger.warn).not.toHaveBeenCalledWith(
+          expect.stringContaining(`Invalid hook event name: ${field}`),
+        );
+      }
+
+      // Should warn on truly invalid event name
+      expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith(
+        'warning',
+        expect.stringContaining('Invalid hook event name: "InvalidEvent"'),
+      );
+    });
   });
 
   describe('project hook warnings', () => {

packages/core/src/hooks/hookRegistry.ts

@@ -6,7 +6,7 @@
 
 import type { Config } from '../config/config.js';
 import type { HookDefinition, HookConfig } from './types.js';
-import { HookEventName, ConfigSource } from './types.js';
+import { HookEventName, ConfigSource, HOOKS_CONFIG_FIELDS } from './types.js';
 import { debugLogger } from '../utils/debugLogger.js';
 import { TrustedHooksManager } from './trustedHooks.js';
 import { coreEvents } from '../utils/events.js';
@@ -169,8 +169,15 @@ please review the project settings (.gemini/settings.json) and remove them.`;
     source: ConfigSource,
   ): void {
     for (const [eventName, definitions] of Object.entries(hooksConfig)) {
+      if (HOOKS_CONFIG_FIELDS.includes(eventName)) {
+        continue;
+      }
+
       if (!this.isValidEventName(eventName)) {
-        debugLogger.warn(`Invalid hook event name: ${eventName}`);
+        coreEvents.emitFeedback(
+          'warning',
+          `Invalid hook event name: "${eventName}" from ${source} config. Skipping.`,
+        );
         continue;
       }
 

packages/core/src/hooks/types.ts

@@ -44,6 +44,11 @@ export enum HookEventName {
   BeforeToolSelection = 'BeforeToolSelection',
 }
 
+/**
+ * Fields in the hooks configuration that are not hook event names
+ */
+export const HOOKS_CONFIG_FIELDS = ['enabled', 'disabled', 'notifications'];
+
 /**
  * Hook configuration entry
  */