feat(core): Ensure all properties in hooks object are event names. (#16870)

packages/cli/src/commands/hooks/migrate.test.ts

@@ -512,7 +512,7 @@ describe('migrate command', () => {
       '\nMigration complete! Please review the migrated hooks in .gemini/settings.json',
     );
     expect(debugLoggerLogSpy).toHaveBeenCalledWith(
-      'Note: Set hooks.enabled to true in your settings to enable the hook system.',
+      'Note: Set hooksConfig.enabled to true in your settings to enable the hook system.',
     );
   });
 });

packages/cli/src/commands/hooks/migrate.ts

@@ -230,7 +230,10 @@ export async function handleMigrateFromClaude() {
   const settings = loadSettings(workingDir);
 
   // Merge migrated hooks with existing hooks
-  const existingHooks = settings.merged.hooks as Record<string, unknown>;
+  const existingHooks = (settings.merged?.hooks || {}) as Record<
+    string,
+    unknown
+  >;
   const mergedHooks = { ...existingHooks, ...migratedHooks };
 
   // Update settings (setValue automatically saves)
@@ -242,7 +245,7 @@ export async function handleMigrateFromClaude() {
       '\nMigration complete! Please review the migrated hooks in .gemini/settings.json',
     );
     debugLogger.log(
-      'Note: Set hooks.enabled to true in your settings to enable the hook system.',
+      'Note: Set hooksConfig.enabled to true in your settings to enable the hook system.',
     );
   } catch (error) {
     debugLogger.error(`Error saving migrated hooks: ${getErrorMessage(error)}`);

packages/cli/src/config/config.ts

@@ -763,9 +763,10 @@ export async function loadCliConfig(
     // TODO: loading of hooks based on workspace trust
     enableHooks:
       (settings.tools?.enableHooks ?? true) &&
-      (settings.hooks?.enabled ?? false),
+      (settings.hooksConfig?.enabled ?? false),
     enableHooksUI: settings.tools?.enableHooks ?? true,
     hooks: settings.hooks || {},
+    disabledHooks: settings.hooksConfig?.disabled || [],
     projectHooks: projectHooks || {},
     onModelChange: (model: string) => saveModelChange(loadedSettings, model),
     onReload: async () => {

packages/cli/src/config/extension-manager.ts

@@ -613,7 +613,10 @@ Would you like to attempt to install via "git clone" instead?`,
         .filter((contextFilePath) => fs.existsSync(contextFilePath));
 
       let hooks: { [K in HookEventName]?: HookDefinition[] } | undefined;
-      if (this.settings.tools.enableHooks && this.settings.hooks.enabled) {
+      if (
+        this.settings.tools.enableHooks &&
+        this.settings.hooksConfig.enabled
+      ) {
         hooks = await this.loadExtensionHooks(effectiveExtensionPath, {
           extensionPath: effectiveExtensionPath,
           workspacePath: this.workspaceDir,

packages/cli/src/config/extension.test.ts

@@ -815,6 +815,7 @@ describe('extension tests', () => {
         fs.mkdirSync(hooksDir);
 
         const hooksConfig = {
+          enabled: false,
           hooks: {
             BeforeTool: [
               {
@@ -836,7 +837,7 @@ describe('extension tests', () => {
         );
 
         const settings = loadSettings(tempWorkspaceDir).merged;
-        settings.hooks.enabled = true;
+        settings.hooksConfig.enabled = true;
 
         extensionManager = new ExtensionManager({
           workspaceDir: tempWorkspaceDir,
@@ -867,11 +868,10 @@ describe('extension tests', () => {
         fs.mkdirSync(hooksDir);
         fs.writeFileSync(
           path.join(hooksDir, 'hooks.json'),
-          JSON.stringify({ hooks: { BeforeTool: [] } }),
+          JSON.stringify({ hooks: { BeforeTool: [] }, enabled: false }),
         );
 
         const settings = loadSettings(tempWorkspaceDir).merged;
-        settings.hooks.enabled = false;
 
         extensionManager = new ExtensionManager({
           workspaceDir: tempWorkspaceDir,

packages/cli/src/config/settingsSchema.test.ts

@@ -395,8 +395,8 @@ describe('SettingsSchema', () => {
       );
     });
 
-    it('should have hooks.notifications setting in schema', () => {
-      const setting = getSettingsSchema().hooks.properties.notifications;
+    it('should have hooksConfig.notifications setting in schema', () => {
+      const setting = getSettingsSchema().hooksConfig?.properties.notifications;
       expect(setting).toBeDefined();
       expect(setting.type).toBe('boolean');
       expect(setting.category).toBe('Advanced');

packages/cli/src/config/settingsSchema.ts

@@ -1631,9 +1631,9 @@ const SETTINGS_SCHEMA = {
     },
   },
 
-  hooks: {
+  hooksConfig: {
     type: 'object',
-    label: 'Hooks',
+    label: 'HooksConfig',
     category: 'Advanced',
     requiresRestart: false,
     default: {},
@@ -1675,6 +1675,18 @@ const SETTINGS_SCHEMA = {
         description: 'Show visual indicators when hooks are executing.',
         showInDialog: true,
       },
+    },
+  },
+
+  hooks: {
+    type: 'object',
+    label: 'Hook Events',
+    category: 'Advanced',
+    requiresRestart: false,
+    default: {},
+    description: 'Event-specific hook configurations.',
+    showInDialog: false,
+    properties: {
       BeforeTool: {
         type: 'array',
         label: 'Before Tool Hooks',

packages/cli/src/ui/commands/hooksCommand.test.ts

@@ -26,7 +26,7 @@ describe('hooksCommand', () => {
   };
   let mockSettings: {
     merged: {
-      hooks?: {
+      hooksConfig?: {
         disabled?: string[];
       };
       tools?: {
@@ -58,7 +58,7 @@ describe('hooksCommand', () => {
     // Create mock settings
     mockSettings = {
       merged: {
-        hooks: {
+        hooksConfig: {
           disabled: [],
         },
       },
@@ -273,7 +273,7 @@ describe('hooksCommand', () => {
 
     it('should enable a hook and update settings', async () => {
       // Update the context's settings with disabled hooks
-      mockContext.services.settings.merged.hooks.disabled = [
+      mockContext.services.settings.merged.hooksConfig.disabled = [
         'test-hook',
         'other-hook',
       ];
@@ -289,7 +289,7 @@ describe('hooksCommand', () => {
 
       expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
         expect.any(String),
-        'hooks.disabled',
+        'hooksConfig.disabled',
         ['other-hook'],
       );
       expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(
@@ -404,7 +404,7 @@ describe('hooksCommand', () => {
     });
 
     it('should disable a hook and update settings', async () => {
-      mockContext.services.settings.merged.hooks.disabled = [];
+      mockContext.services.settings.merged.hooksConfig.disabled = [];
 
       const disableCmd = hooksCommand.subCommands!.find(
         (cmd) => cmd.name === 'disable',
@@ -417,7 +417,7 @@ describe('hooksCommand', () => {
 
       expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
         expect.any(String),
-        'hooks.disabled',
+        'hooksConfig.disabled',
         ['test-hook'],
       );
       expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(
@@ -433,7 +433,7 @@ describe('hooksCommand', () => {
 
     it('should synchronize with hook system even if hook is already in disabled list', async () => {
       // Update the context's settings with the hook already disabled
-      mockContext.services.settings.merged.hooks.disabled = ['test-hook'];
+      mockContext.services.settings.merged.hooksConfig.disabled = ['test-hook'];
 
       const disableCmd = hooksCommand.subCommands!.find(
         (cmd) => cmd.name === 'disable',
@@ -458,7 +458,7 @@ describe('hooksCommand', () => {
     });
 
     it('should handle error when disabling hook fails', async () => {
-      mockContext.services.settings.merged.hooks.disabled = [];
+      mockContext.services.settings.merged.hooksConfig.disabled = [];
       mockSettings.setValue.mockImplementationOnce(() => {
         throw new Error('Failed to save settings');
       });
@@ -637,7 +637,7 @@ describe('hooksCommand', () => {
 
       expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
         expect.any(String),
-        'hooks.disabled',
+        'hooksConfig.disabled',
         [],
       );
       expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(
@@ -761,7 +761,7 @@ describe('hooksCommand', () => {
 
       expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
         expect.any(String),
-        'hooks.disabled',
+        'hooksConfig.disabled',
         ['hook-1', 'hook-2', 'hook-3'],
       );
       expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(

packages/cli/src/ui/commands/hooksCommand.ts

@@ -76,7 +76,7 @@ async function enableAction(
 
   // Get current disabled hooks from settings
   const settings = context.services.settings;
-  const disabledHooks = settings.merged.hooks.disabled;
+  const disabledHooks = settings.merged.hooksConfig.disabled;
   // Remove from disabled list if present
   const newDisabledHooks = disabledHooks.filter(
     (name: string) => name !== hookName,
@@ -87,10 +87,10 @@ async function enableAction(
     const scope = settings.workspace
       ? SettingScope.Workspace
       : SettingScope.User;
-    settings.setValue(scope, 'hooks.disabled', newDisabledHooks);
+    settings.setValue(scope, 'hooksConfig.disabled', newDisabledHooks);
 
     // Update core config so re-initialization (e.g. extension reload) respects the change
-    config.updateDisabledHooks(settings.merged.hooks.disabled);
+    config.updateDisabledHooks(settings.merged.hooksConfig.disabled);
 
     // Enable in hook system
     hookSystem.setHookEnabled(hookName, true);
@@ -145,7 +145,7 @@ async function disableAction(
 
   // Get current disabled hooks from settings
   const settings = context.services.settings;
-  const disabledHooks = settings.merged.hooks.disabled;
+  const disabledHooks = settings.merged.hooksConfig.disabled;
   // Add to disabled list if not already present
   try {
     if (!disabledHooks.includes(hookName)) {
@@ -154,11 +154,11 @@ async function disableAction(
       const scope = settings.workspace
         ? SettingScope.Workspace
         : SettingScope.User;
-      settings.setValue(scope, 'hooks.disabled', newDisabledHooks);
+      settings.setValue(scope, 'hooksConfig.disabled', newDisabledHooks);
     }
 
     // Update core config so re-initialization (e.g. extension reload) respects the change
-    config.updateDisabledHooks(settings.merged.hooks.disabled);
+    config.updateDisabledHooks(settings.merged.hooksConfig.disabled);
 
     // Always disable in hook system to ensure in-memory state matches settings
     hookSystem.setHookEnabled(hookName, false);
@@ -250,10 +250,10 @@ async function enableAllAction(
     const scope = settings.workspace
       ? SettingScope.Workspace
       : SettingScope.User;
-    settings.setValue(scope, 'hooks.disabled', []);
+    settings.setValue(scope, 'hooksConfig.disabled', []);
 
     // Update core config so re-initialization (e.g. extension reload) respects the change
-    config.updateDisabledHooks(settings.merged.hooks.disabled);
+    config.updateDisabledHooks(settings.merged.hooksConfig.disabled);
 
     for (const hook of disabledHooks) {
       const hookName = getHookDisplayName(hook);
@@ -323,10 +323,10 @@ async function disableAllAction(
     const scope = settings.workspace
       ? SettingScope.Workspace
       : SettingScope.User;
-    settings.setValue(scope, 'hooks.disabled', allHookNames);
+    settings.setValue(scope, 'hooksConfig.disabled', allHookNames);
 
     // Update core config so re-initialization (e.g. extension reload) respects the change
-    config.updateDisabledHooks(settings.merged.hooks.disabled);
+    config.updateDisabledHooks(settings.merged.hooksConfig.disabled);
 
     for (const hook of enabledHooks) {
       const hookName = getHookDisplayName(hook);

packages/cli/src/ui/components/StatusDisplay.test.tsx

@@ -52,7 +52,7 @@ const createMockConfig = (overrides = {}) => ({
 
 const createMockSettings = (merged = {}) => ({
   merged: {
-    hooks: { notifications: true },
+    hooksConfig: { notifications: true },
     ui: { hideContextSummary: false },
     ...merged,
   },
@@ -185,7 +185,7 @@ describe('StatusDisplay', () => {
       activeHooks: [{ name: 'hook', eventName: 'event' }],
     });
     const settings = createMockSettings({
-      hooks: { notifications: false },
+      hooksConfig: { notifications: false },
     });
     const { lastFrame } = renderStatusDisplay(
       { hideContextSummary: false },

packages/cli/src/ui/components/StatusDisplay.tsx

@@ -52,7 +52,10 @@ export const StatusDisplay: React.FC<StatusDisplayProps> = ({
     return <Text color={theme.status.error}>{uiState.queueErrorMessage}</Text>;
   }
 
-  if (uiState.activeHooks.length > 0 && settings.merged.hooks.notifications) {
+  if (
+    uiState.activeHooks.length > 0 &&
+    settings.merged.hooksConfig.notifications
+  ) {
     return <HookStatusDisplay activeHooks={uiState.activeHooks} />;
   }