Merge pull request #2781 from QwenLM/feat/hooks-remove-experimental

feat(hooks): remove experimental flag and add disabled state UI

packages/core/src/config/config.test.ts

@@ -347,7 +347,7 @@ describe('Server Config (config.ts)', () => {
       const mockMessageBus = { request: vi.fn() };
       const config = new Config({
         ...baseParams,
-        enableHooks: true,
+        disableAllHooks: false,
       });
       // Set messageBus using the setter
       config.setMessageBus(mockMessageBus as unknown as MessageBus);
@@ -378,7 +378,7 @@ describe('Server Config (config.ts)', () => {
     it('should not fire notification hook when hooks are disabled', async () => {
       const config = new Config({
         ...baseParams,
-        enableHooks: false,
+        disableAllHooks: true,
       });
       const authType = AuthType.USE_GEMINI;
       const mockContentConfig = {

packages/core/src/config/config.ts

@@ -419,12 +419,15 @@ export interface ConfigParameters {
   modelProvidersConfig?: ModelProvidersConfig;
   /** Multi-agent collaboration settings (Arena, Team, Swarm) */
   agents?: AgentsCollabSettings;
-  /** Enable hook system for lifecycle events */
-  enableHooks?: boolean;
+  /**
+   * Disable all hooks (default: false, hooks enabled).
+   * Migration note: This replaces the deprecated hooksConfig.enabled setting.
+   * Users with old settings.json containing hooksConfig.enabled should migrate
+   * to use disableAllHooks instead (note: inverted logic - enabled:true → disableAllHooks:false).
+   */
+  disableAllHooks?: boolean;
   /** Hooks configuration from settings */
   hooks?: Record<string, unknown>;
-  /** Hooks config settings (enabled, disabled list) */
-  hooksConfig?: Record<string, unknown>;
   /** Warnings generated during configuration resolution */
   warnings?: string[];
   /**
@@ -594,9 +597,8 @@ export class Config {
   private readonly eventEmitter?: EventEmitter;
   private readonly channel: string | undefined;
   private readonly defaultFileEncoding: FileEncodingType | undefined;
-  private readonly enableHooks: boolean;
+  private readonly disableAllHooks: boolean;
   private readonly hooks?: Record<string, unknown>;
-  private readonly hooksConfig?: Record<string, unknown>;
   private hookSystem?: HookSystem;
   private messageBus?: MessageBus;
 
@@ -761,9 +763,8 @@ export class Config {
       enabledExtensionOverrides: this.overrideExtensions,
       isWorkspaceTrusted: this.isTrustedFolder(),
     });
-    this.enableHooks = params.enableHooks ?? false;
+    this.disableAllHooks = params.disableAllHooks ?? false;
     this.hooks = params.hooks;
-    this.hooksConfig = params.hooksConfig;
   }
 
   /**
@@ -788,7 +789,7 @@ export class Config {
     this.debugLogger.debug('Extension manager initialized');
 
     // Initialize hook system if enabled
-    if (this.enableHooks) {
+    if (!this.disableAllHooks) {
       this.hookSystem = new HookSystem(this);
       await this.hookSystem.initialize();
       this.debugLogger.debug('Hook system initialized');
@@ -1074,7 +1075,7 @@ export class Config {
 
     // Fire auth_success notification hook (supports both interactive & non-interactive)
     const messageBus = this.getMessageBus();
-    const hooksEnabled = this.getEnableHooks();
+    const hooksEnabled = !this.getDisableAllHooks();
     if (hooksEnabled && messageBus) {
       fireNotificationHook(
         messageBus,
@@ -1781,10 +1782,10 @@ export class Config {
   }
 
   /**
-   * Check if hooks are enabled.
+   * Check if all hooks are disabled.
    */
-  getEnableHooks(): boolean {
-    return this.enableHooks;
+  getDisableAllHooks(): boolean {
+    return this.disableAllHooks;
   }
 
   /**
@@ -1803,17 +1804,6 @@ export class Config {
     this.messageBus = messageBus;
   }
 
-  /**
-   * Get the list of disabled hook names.
-   * This is used by the HookRegistry to filter out disabled hooks.
-   */
-  getDisabledHooks(): string[] {
-    const hooksConfig = this.hooksConfig;
-    if (!hooksConfig) return [];
-    const disabled = hooksConfig['disabled'];
-    return Array.isArray(disabled) ? (disabled as string[]) : [];
-  }
-
   /**
    * Get project-level hooks configuration.
    * This is used by the HookRegistry to load project-specific hooks.

packages/core/src/core/client.test.ts

@@ -360,7 +360,7 @@ describe('Gemini Client (client.ts)', () => {
       getChatRecordingService: vi.fn().mockReturnValue(undefined),
       getResumedSessionData: vi.fn().mockReturnValue(undefined),
       getArenaAgentClient: vi.fn().mockReturnValue(null),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
       getArenaManager: vi.fn().mockReturnValue(null),
       getMessageBus: vi.fn().mockReturnValue(undefined),
       hasHooksForEvent: vi.fn().mockReturnValue(false),
@@ -2415,11 +2415,11 @@ Other open files:
           request: vi.fn(),
           response: vi.fn(),
         };
-        vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
-        vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
+        vi.mocked(mockConfig.getDisableAllHooks).mockReturnValue(false);
+        vi.mocked(mockConfig.getMessageBus).mockReturnValue(
           mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
         );
-        vi.spyOn(client['config'], 'hasHooksForEvent').mockReturnValue(false);
+        vi.mocked(mockConfig.hasHooksForEvent).mockReturnValue(false);
 
         const stream = client.sendMessageStream(
           [{ text: 'Hi' }],
@@ -2439,11 +2439,11 @@ Other open files:
           request: vi.fn(),
           response: vi.fn(),
         };
-        vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
-        vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
+        vi.mocked(mockConfig.getDisableAllHooks).mockReturnValue(false);
+        vi.mocked(mockConfig.getMessageBus).mockReturnValue(
           mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
         );
-        vi.spyOn(client['config'], 'hasHooksForEvent').mockReturnValue(false);
+        vi.mocked(mockConfig.hasHooksForEvent).mockReturnValue(false);
 
         const stream = client.sendMessageStream(
           [{ text: 'Hi' }],
@@ -2463,11 +2463,11 @@ Other open files:
           request: vi.fn().mockResolvedValue({ modifiedPrompt: undefined }),
           response: vi.fn(),
         };
-        vi.spyOn(client['config'], 'getEnableHooks').mockReturnValue(true);
-        vi.spyOn(client['config'], 'getMessageBus').mockReturnValue(
+        vi.mocked(mockConfig.getDisableAllHooks).mockReturnValue(false);
+        vi.mocked(mockConfig.getMessageBus).mockReturnValue(
           mockMessageBus as unknown as ReturnType<Config['getMessageBus']>,
         );
-        vi.spyOn(client['config'], 'hasHooksForEvent').mockImplementation(
+        vi.mocked(mockConfig.hasHooksForEvent).mockImplementation(
           (event: string) => event === 'UserPromptSubmit',
         );
 

packages/core/src/core/client.ts

@@ -463,7 +463,7 @@ export class GeminiClient {
     }
 
     // Fire UserPromptSubmit hook through MessageBus (only if hooks are enabled)
-    const hooksEnabled = this.config.getEnableHooks();
+    const hooksEnabled = !this.config.getDisableAllHooks();
     const messageBus = this.config.getMessageBus();
     if (
       messageType !== SendMessageType.Retry &&

packages/core/src/core/coreToolScheduler.test.ts

@@ -266,7 +266,7 @@ describe('CoreToolScheduler', () => {
       getGeminiClient: () => null, // No client needed for these tests
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -345,7 +345,7 @@ describe('CoreToolScheduler', () => {
       getGeminiClient: () => null,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -390,7 +390,7 @@ describe('CoreToolScheduler', () => {
         getPermissionsDeny: () => undefined,
         isInteractive: () => true,
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       // Create scheduler
@@ -433,7 +433,7 @@ describe('CoreToolScheduler', () => {
         getPermissionsDeny: () => ['write_file', 'edit', 'run_shell_command'],
         isInteractive: () => false, // Value doesn't matter, but included for completeness
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       // Create scheduler
@@ -465,7 +465,7 @@ describe('CoreToolScheduler', () => {
         getPermissionsDeny: () => ['write_file', 'edit'],
         isInteractive: () => false, // Value doesn't matter
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       // Create scheduler
@@ -508,7 +508,7 @@ describe('CoreToolScheduler', () => {
         getPermissionsDeny: () => undefined,
         isInteractive: () => true,
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       // Create scheduler
@@ -588,7 +588,7 @@ describe('CoreToolScheduler', () => {
         getGeminiClient: () => null,
         getChatRecordingService: () => undefined,
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       const scheduler = new CoreToolScheduler({
@@ -676,7 +676,7 @@ describe('CoreToolScheduler', () => {
         getGeminiClient: () => null,
         getChatRecordingService: () => undefined,
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       const scheduler = new CoreToolScheduler({
@@ -770,7 +770,7 @@ describe('CoreToolScheduler with payload', () => {
       getExperimentalZedIntegration: () => false,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1112,7 +1112,7 @@ describe('CoreToolScheduler edit cancellation', () => {
       getExperimentalZedIntegration: () => false,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1221,7 +1221,7 @@ describe('CoreToolScheduler YOLO mode', () => {
       getGeminiClient: () => null, // No client needed for these tests
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1364,7 +1364,7 @@ describe('CoreToolScheduler cancellation during executing with live output', ()
       }),
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1466,7 +1466,7 @@ describe('CoreToolScheduler request queueing', () => {
       getGeminiClient: () => null, // No client needed for these tests
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1590,7 +1590,7 @@ describe('CoreToolScheduler request queueing', () => {
       getGeminiClient: () => null, // No client needed for these tests
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -1667,7 +1667,7 @@ describe('CoreToolScheduler request queueing', () => {
       getExperimentalZedIntegration: () => false,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const testTool = new TestApprovalTool(mockConfig);
@@ -1832,7 +1832,7 @@ describe('CoreToolScheduler truncated output protection', () => {
       getChatRecordingService: () => undefined,
       isInteractive: () => true,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -2031,7 +2031,7 @@ describe('CoreToolScheduler Sequential Execution', () => {
       getGeminiClient: () => null,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -2153,7 +2153,7 @@ describe('CoreToolScheduler Sequential Execution', () => {
       getGeminiClient: () => null,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     const scheduler = new CoreToolScheduler({
@@ -2329,7 +2329,7 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => {
       getExperimentalZedIntegration: () => false,
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     return new CoreToolScheduler({
@@ -2972,7 +2972,7 @@ describe('Fire hook functions integration', () => {
         getGeminiClient: () => null,
         getChatRecordingService: () => undefined,
         getMessageBus: vi.fn().mockReturnValue(undefined),
-        getEnableHooks: vi.fn().mockReturnValue(false),
+        getDisableAllHooks: vi.fn().mockReturnValue(true),
       } as unknown as Config;
 
       return new CoreToolScheduler({

packages/core/src/core/coreToolScheduler.ts

@@ -1009,7 +1009,7 @@ export class CoreToolScheduler {
             const messageBus = this.config.getMessageBus() as
               | MessageBus
               | undefined;
-            const hooksEnabled = this.config.getEnableHooks();
+            const hooksEnabled = !this.config.getDisableAllHooks();
 
             if (hooksEnabled && messageBus) {
               const permissionMode = String(this.config.getApprovalMode());
@@ -1326,7 +1326,7 @@ export class CoreToolScheduler {
 
     // Get MessageBus for hook execution
     const messageBus = this.config.getMessageBus() as MessageBus | undefined;
-    const hooksEnabled = this.config.getEnableHooks();
+    const hooksEnabled = !this.config.getDisableAllHooks();
 
     // PreToolUse Hook
     if (hooksEnabled && messageBus) {

packages/core/src/core/nonInteractiveToolExecutor.test.ts

@@ -63,7 +63,7 @@ describe('executeToolCall', () => {
       getGeminiClient: () => null, // No client needed for these tests
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(undefined),
-      getEnableHooks: vi.fn().mockReturnValue(false),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
       getHookSystem: vi.fn().mockReturnValue(undefined),
       getDebugLogger: vi.fn().mockReturnValue({
         debug: vi.fn(),

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

@@ -28,7 +28,6 @@ describe('HookRegistry', () => {
       isTrustedFolder: vi.fn().mockReturnValue(true),
       getHooks: vi.fn().mockReturnValue(undefined),
       getProjectHooks: vi.fn().mockReturnValue(undefined),
-      getDisabledHooks: vi.fn().mockReturnValue([]),
       getExtensions: vi.fn().mockReturnValue([]),
     };
     mockFeedbackEmitter = {
@@ -123,21 +122,20 @@ describe('HookRegistry', () => {
       expect(postHooks[0].config.name).toBe('post-hook');
     });
 
-    it('should filter out disabled hooks', async () => {
-      mockConfig.getDisabledHooks = vi.fn().mockReturnValue(['disabled-hook']);
+    it('should register all hooks as enabled by default', async () => {
       const hooksConfig = {
         [HookEventName.PreToolUse]: [
           {
             hooks: [
               {
                 type: HookType.Command,
-                command: 'echo enabled',
-                name: 'enabled-hook',
+                command: 'echo first',
+                name: 'first-hook',
               },
               {
                 type: HookType.Command,
-                command: 'echo disabled',
-                name: 'disabled-hook',
+                command: 'echo second',
+                name: 'second-hook',
               },
             ],
           },
@@ -149,8 +147,9 @@ describe('HookRegistry', () => {
       await registry.initialize();
 
       const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
-      expect(hooks).toHaveLength(1);
-      expect(hooks[0].config.name).toBe('enabled-hook');
+      expect(hooks).toHaveLength(2);
+      expect(hooks[0].enabled).toBe(true);
+      expect(hooks[1].enabled).toBe(true);
     });
 
     it('should sort hooks by source priority', async () => {
@@ -181,37 +180,6 @@ describe('HookRegistry', () => {
   });
 
   describe('setHookEnabled', () => {
-    it('should enable a disabled hook', async () => {
-      mockConfig.getDisabledHooks = vi.fn().mockReturnValue(['test-hook']);
-      const hooksConfig = {
-        [HookEventName.PreToolUse]: [
-          {
-            hooks: [
-              {
-                type: HookType.Command,
-                command: 'echo test',
-                name: 'test-hook',
-              },
-            ],
-          },
-        ],
-      };
-      mockConfig.getHooks = vi.fn().mockReturnValue(hooksConfig);
-
-      const registry = new HookRegistry(mockConfig);
-      await registry.initialize();
-
-      expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
-        0,
-      );
-
-      registry.setHookEnabled('test-hook', true);
-
-      const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
-      expect(hooks).toHaveLength(1);
-      expect(hooks[0].enabled).toBe(true);
-    });
-
     it('should disable an enabled hook', async () => {
       const hooksConfig = {
         [HookEventName.PreToolUse]: [
@@ -237,9 +205,40 @@ describe('HookRegistry', () => {
 
       registry.setHookEnabled('test-hook', false);
 
+      const hooks = registry.getHooksForEvent(HookEventName.PreToolUse);
+      expect(hooks).toHaveLength(0);
+    });
+
+    it('should enable a disabled hook', async () => {
+      const hooksConfig = {
+        [HookEventName.PreToolUse]: [
+          {
+            hooks: [
+              {
+                type: HookType.Command,
+                command: 'echo test',
+                name: 'test-hook',
+              },
+            ],
+          },
+        ],
+      };
+      mockConfig.getHooks = vi.fn().mockReturnValue(hooksConfig);
+
+      const registry = new HookRegistry(mockConfig);
+      await registry.initialize();
+
+      // First disable the hook
+      registry.setHookEnabled('test-hook', false);
       expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
         0,
       );
+
+      // Then enable it again
+      registry.setHookEnabled('test-hook', true);
+      expect(registry.getHooksForEvent(HookEventName.PreToolUse)).toHaveLength(
+        1,
+      );
     });
 
     it('should update all hooks with matching name', async () => {

packages/core/src/hooks/hookRegistry.ts

@@ -32,7 +32,6 @@ export interface HookRegistryConfig {
   isTrustedFolder(): boolean;
   getHooks(): { [K in HookEventName]?: HookDefinition[] } | undefined;
   getProjectHooks(): { [K in HookEventName]?: HookDefinition[] } | undefined;
-  getDisabledHooks(): string[];
   getExtensions(): ExtensionWithHooks[];
 }
 
@@ -250,18 +249,13 @@ please review the project settings (.qwen/settings.json) and remove them.`;
       return;
     }
 
-    // Get disabled hooks list from settings
-    const disabledHooks = this.config.getDisabledHooks();
-
     for (const hookConfig of definition.hooks) {
       if (
         hookConfig &&
         typeof hookConfig === 'object' &&
         this.validateHookConfig(hookConfig, eventName, source)
       ) {
-        // Check if this hook is in the disabled list
         const hookName = this.getHookName({ config: hookConfig });
-        const isDisabled = disabledHooks.includes(hookName);
 
         // Check for duplicate hooks (same name+command+source+eventName+matcher+sequential)
         const isDuplicate = this.entries.some(
@@ -288,7 +282,7 @@ please review the project settings (.qwen/settings.json) and remove them.`;
           eventName,
           matcher: definition.matcher,
           sequential: definition.sequential,
-          enabled: !isDisabled,
+          enabled: true,
         });
       } else {
         // Invalid hooks are logged and discarded here, they won't reach HookRunner