Improve hooks matcher display

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

@@ -17,7 +17,6 @@ describe('hooksCommand', () => {
   beforeEach(() => {
     vi.clearAllMocks();
 
-    // Create mock config with hook system
     mockConfig = {
       getHookSystem: vi.fn().mockReturnValue({
         getRegistry: vi.fn().mockReturnValue({
@@ -69,4 +68,173 @@ describe('hooksCommand', () => {
       });
     });
   });
+
+  describe('non-interactive list output', () => {
+    function makeContext(opts: {
+      configHooks: Array<{
+        eventName: string;
+        matcher?: string;
+        source: string;
+        config: {
+          type: string;
+          command?: string;
+          url?: string;
+          name?: string;
+        };
+      }>;
+      sessionHooks?: Array<{
+        eventName: string;
+        matcher?: string;
+        config: { type: string; command?: string; name?: string };
+      }>;
+    }) {
+      const sessionConfig = {
+        getHookSystem: vi.fn().mockReturnValue({
+          getRegistry: vi.fn().mockReturnValue({
+            getAllHooks: vi.fn().mockReturnValue(opts.configHooks),
+          }),
+          getSessionHooksManager: vi.fn().mockReturnValue({
+            getAllSessionHooks: vi
+              .fn()
+              .mockReturnValue(opts.sessionHooks ?? []),
+          }),
+        }),
+        getSessionId: vi.fn().mockReturnValue('sid'),
+      };
+      return createMockCommandContext({
+        executionMode: 'non_interactive',
+        services: { config: sessionConfig },
+      });
+    }
+
+    it('groups hooks under matcher headings', async () => {
+      const ctx = makeContext({
+        configHooks: [
+          {
+            eventName: 'PreToolUse',
+            matcher: 'Bash',
+            source: 'user',
+            config: { type: 'command', command: '/check-bash.sh' },
+          },
+          {
+            eventName: 'PreToolUse',
+            matcher: 'Edit|Write',
+            source: 'project',
+            config: { type: 'command', command: '/format.sh' },
+          },
+        ],
+      });
+
+      const result = await hooksCommand.action!(ctx, '');
+      expect(result).toBeDefined();
+      const content = (result as { content: string }).content;
+
+      expect(content).toContain('### PreToolUse');
+      expect(content).toContain('#### Matcher: Bash');
+      expect(content).toContain('/check-bash.sh');
+      expect(content).toContain('#### Matcher: Edit|Write');
+      expect(content).toContain('/format.sh');
+    });
+
+    it('renders missing matcher as *', async () => {
+      const ctx = makeContext({
+        configHooks: [
+          {
+            eventName: 'PreToolUse',
+            source: 'user',
+            config: { type: 'command', command: '/anything.sh' },
+          },
+        ],
+      });
+
+      const result = await hooksCommand.action!(ctx, '');
+      const content = (result as { content: string }).content;
+
+      expect(content).toContain('#### Matcher: *');
+      expect(content).toContain('/anything.sh');
+    });
+
+    it('does not emit a Matcher heading for non-matcher events like Stop', async () => {
+      const ctx = makeContext({
+        configHooks: [
+          {
+            eventName: 'Stop',
+            source: 'user',
+            config: { type: 'command', command: '/stop-hook.sh' },
+          },
+        ],
+      });
+
+      const result = await hooksCommand.action!(ctx, '');
+      const content = (result as { content: string }).content;
+
+      expect(content).toContain('### Stop');
+      expect(content).not.toContain('Matcher:');
+      expect(content).toContain('/stop-hook.sh');
+    });
+
+    it('preserves registration order for non-matcher events with ignored matchers', async () => {
+      const ctx = makeContext({
+        configHooks: [
+          {
+            eventName: 'Stop',
+            matcher: 'A',
+            source: 'user',
+            config: { type: 'command', command: '/first.sh' },
+          },
+          {
+            eventName: 'Stop',
+            matcher: 'B',
+            source: 'user',
+            config: { type: 'command', command: '/second.sh' },
+          },
+          {
+            eventName: 'Stop',
+            matcher: 'A',
+            source: 'user',
+            config: { type: 'command', command: '/third.sh' },
+          },
+        ],
+      });
+
+      const result = await hooksCommand.action!(ctx, '');
+      const content = (result as { content: string }).content;
+
+      expect(content).not.toContain('Matcher:');
+      expect(content.indexOf('/first.sh')).toBeLessThan(
+        content.indexOf('/second.sh'),
+      );
+      expect(content.indexOf('/second.sh')).toBeLessThan(
+        content.indexOf('/third.sh'),
+      );
+    });
+
+    it('groups session hooks by their matcher alongside config hooks', async () => {
+      const ctx = makeContext({
+        configHooks: [
+          {
+            eventName: 'PreToolUse',
+            matcher: 'Bash',
+            source: 'user',
+            config: { type: 'command', command: '/persistent.sh' },
+          },
+        ],
+        sessionHooks: [
+          {
+            eventName: 'PreToolUse',
+            matcher: 'Bash',
+            config: { type: 'command', command: '/session.sh' },
+          },
+        ],
+      });
+
+      const result = await hooksCommand.action!(ctx, '');
+      const content = (result as { content: string }).content;
+
+      const matcherOccurrences = content.match(/#### Matcher: Bash/g) ?? [];
+      expect(matcherOccurrences).toHaveLength(1);
+      expect(content).toContain('/persistent.sh');
+      expect(content).toContain('/session.sh');
+    });
+  });
 });

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

@@ -15,7 +15,10 @@ import { t } from '../../i18n/index.js';
 import type {
   HookRegistryEntry,
   SessionHookEntry,
+  HookEventName,
 } from '@qwen-code/qwen-code-core';
+import { supportsMatchers } from '../components/hooks/constants.js';
+import { normalizeMatcher } from '../components/hooks/matcherGrouping.js';
 
 /**
  * Format hook source for display
@@ -71,7 +74,6 @@ const listCommand: SlashCommand = {
     const registry = hookSystem.getRegistry();
     const configHooks = registry.getAllHooks();
 
-    // Get session hooks
     const sessionId = config.getSessionId();
     const sessionHooksManager = hookSystem.getSessionHooksManager();
     const sessionHooks = sessionId
@@ -90,86 +92,94 @@ const listCommand: SlashCommand = {
       };
     }
 
-    // Group hooks by event
-    const hooksByEvent = new Map<
-      string,
-      Array<{ hook: HookRegistryEntry | SessionHookEntry; isSession: boolean }>
-    >();
+    interface FlattenedHook {
+      name: string;
+      source: string;
+    }
 
-    // Add config hooks
-    for (const hook of configHooks) {
-      const eventName = hook.eventName;
-      if (!hooksByEvent.has(eventName)) {
-        hooksByEvent.set(eventName, []);
+    const hooksByEvent = new Map<string, Map<string, FlattenedHook[]>>();
+
+    const addHook = (
+      eventName: string,
+      matcher: string,
+      hook: FlattenedHook,
+    ): void => {
+      const matcherKey = supportsMatchers(eventName as HookEventName)
+        ? matcher
+        : '*';
+      let matcherMap = hooksByEvent.get(eventName);
+      if (!matcherMap) {
+        matcherMap = new Map<string, FlattenedHook[]>();
+        hooksByEvent.set(eventName, matcherMap);
+      }
+      let bucket = matcherMap.get(matcherKey);
+      if (!bucket) {
+        bucket = [];
+        matcherMap.set(matcherKey, bucket);
       }
-      hooksByEvent.get(eventName)!.push({ hook, isSession: false });
+      bucket.push(hook);
+    };
+
+    const extractName = (config: {
+      type: string;
+      command?: string;
+      url?: string;
+      name?: string;
+    }): string =>
+      config.name ||
+      (config.type === 'command' ? config.command : undefined) ||
+      (config.type === 'http' ? config.url : undefined) ||
+      'unnamed';
+
+    for (const hook of configHooks) {
+      const configHook = hook as HookRegistryEntry;
+      const config = configHook.config as {
+        type: string;
+        command?: string;
+        url?: string;
+        name?: string;
+      };
+      addHook(configHook.eventName, normalizeMatcher(configHook.matcher), {
+        name: extractName(config),
+        source: formatHookSource(configHook.source),
+      });
     }
 
-    // Add session hooks
     for (const hook of sessionHooks) {
-      const eventName = hook.eventName;
-      if (!hooksByEvent.has(eventName)) {
-        hooksByEvent.set(eventName, []);
-      }
-      hooksByEvent.get(eventName)!.push({ hook, isSession: true });
+      const sessionHook = hook as SessionHookEntry;
+      const config = sessionHook.config as {
+        type: string;
+        command?: string;
+        url?: string;
+        name?: string;
+      };
+      addHook(sessionHook.eventName, normalizeMatcher(sessionHook.matcher), {
+        name: extractName(config),
+        source: formatHookSource('session'),
+      });
     }
 
     let output = `**Configured Hooks (${totalHooks} total)**\n\n`;
 
-    for (const [eventName, hooks] of hooksByEvent) {
-      output += `### ${eventName}\n`;
-      for (const { hook, isSession } of hooks) {
-        let name: string;
-        let source: string;
-        let matcher: string;
-        let config: {
-          type: string;
-          command?: string;
-          url?: string;
-          name?: string;
-        };
-
-        if (isSession) {
-          // Session hook
-          const sessionHook = hook as SessionHookEntry;
-          config = sessionHook.config as {
-            type: string;
-            command?: string;
-            url?: string;
-            name?: string;
-          };
-          name =
-            config.name ||
-            (config.type === 'command' ? config.command : undefined) ||
-            (config.type === 'http' ? config.url : undefined) ||
-            'unnamed';
-          source = formatHookSource('session');
-          matcher = sessionHook.matcher
-            ? ` (matcher: ${sessionHook.matcher})`
-            : '';
-        } else {
-          // Config hook
-          const configHook = hook as HookRegistryEntry;
-          config = configHook.config as {
-            type: string;
-            command?: string;
-            url?: string;
-            name?: string;
-          };
-          name =
-            config.name ||
-            (config.type === 'command' ? config.command : undefined) ||
-            (config.type === 'http' ? config.url : undefined) ||
-            'unnamed';
-          source = formatHookSource(configHook.source);
-          matcher = configHook.matcher
-            ? ` (matcher: ${configHook.matcher})`
-            : '';
+    for (const [eventName, matcherMap] of hooksByEvent) {
+      output += `### ${eventName}\n\n`;
+      const useMatchers = supportsMatchers(eventName as HookEventName);
+      if (useMatchers) {
+        for (const [matcher, hookList] of matcherMap) {
+          output += `#### ${t('Matcher:')} ${matcher}\n`;
+          for (const hook of hookList) {
+            output += `- **${hook.name}** [${hook.source}]\n`;
+          }
+          output += '\n';
         }
-
-        output += `- **${name}** [${source}]${matcher}\n`;
+      } else {
+        for (const hookList of matcherMap.values()) {
+          for (const hook of hookList) {
+            output += `- **${hook.name}** [${hook.source}]\n`;
+          }
+        }
+        output += '\n';
       }
-      output += '\n';
     }
 
     return {
@@ -191,7 +201,6 @@ export const hooksCommand: SlashCommand = {
     context: CommandContext,
     args: string,
   ): Promise<SlashCommandActionReturn> => {
-    // In interactive mode, open the hooks dialog
     const executionMode = context.executionMode ?? 'interactive';
     if (executionMode === 'interactive') {
       return {
@@ -200,7 +209,6 @@ export const hooksCommand: SlashCommand = {
       };
     }
 
-    // In non-interactive mode, list hooks
     const result = await listCommand.action?.(context, args);
     return result ?? { type: 'message', messageType: 'info', content: '' };
   },