From 1361d7d04e7e6cbc7419600215437b18ecb91314 Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 26 May 2026 17:50:41 +0800 Subject: [PATCH 1/2] feat(cli): improve hooks matcher display --- .../cli/src/ui/commands/hooksCommand.test.ts | 170 +++++++- packages/cli/src/ui/commands/hooksCommand.ts | 156 +++---- .../components/hooks/HandlerListBody.test.tsx | 267 ++++++++++++ .../ui/components/hooks/HandlerListBody.tsx | 106 +++++ .../hooks/HookConfigDetailStep.test.tsx | 76 +++- .../components/hooks/HookConfigDetailStep.tsx | 49 +-- .../components/hooks/HookDetailStep.test.tsx | 391 +++++++++++------- .../ui/components/hooks/HookDetailStep.tsx | 170 +------- .../hooks/HookEventHandlerListStep.tsx | 56 +++ .../ui/components/hooks/HookEventHeader.tsx | 65 +++ .../hooks/HookEventMatcherListStep.tsx | 95 +++++ .../hooks/HookMatcherDetailStep.test.tsx | 252 +++++++++++ .../hooks/HookMatcherDetailStep.tsx | 54 +++ .../components/hooks/HooksListStep.test.tsx | 35 +- .../src/ui/components/hooks/HooksListStep.tsx | 14 +- .../hooks/HooksManagementDialog.test.tsx | 164 +++----- .../hooks/HooksManagementDialog.tsx | 241 +++++++---- .../src/ui/components/hooks/constants.test.ts | 96 ++++- .../cli/src/ui/components/hooks/constants.ts | 46 ++- .../components/hooks/matcherGrouping.test.ts | 244 +++++++++++ .../ui/components/hooks/matcherGrouping.ts | 50 +++ .../ui/components/hooks/sourceLabels.test.ts | 115 ++++++ .../src/ui/components/hooks/sourceLabels.ts | 44 ++ packages/cli/src/ui/components/hooks/types.ts | 23 +- packages/core/src/hooks/hookPlanner.ts | 5 + packages/core/src/hooks/index.ts | 2 +- packages/core/src/index.ts | 6 +- 27 files changed, 2321 insertions(+), 671 deletions(-) create mode 100644 packages/cli/src/ui/components/hooks/HandlerListBody.test.tsx create mode 100644 packages/cli/src/ui/components/hooks/HandlerListBody.tsx create mode 100644 packages/cli/src/ui/components/hooks/HookEventHandlerListStep.tsx create mode 100644 packages/cli/src/ui/components/hooks/HookEventHeader.tsx create mode 100644 packages/cli/src/ui/components/hooks/HookEventMatcherListStep.tsx create mode 100644 packages/cli/src/ui/components/hooks/HookMatcherDetailStep.test.tsx create mode 100644 packages/cli/src/ui/components/hooks/HookMatcherDetailStep.tsx create mode 100644 packages/cli/src/ui/components/hooks/matcherGrouping.test.ts create mode 100644 packages/cli/src/ui/components/hooks/matcherGrouping.ts create mode 100644 packages/cli/src/ui/components/hooks/sourceLabels.test.ts create mode 100644 packages/cli/src/ui/components/hooks/sourceLabels.ts diff --git a/packages/cli/src/ui/commands/hooksCommand.test.ts b/packages/cli/src/ui/commands/hooksCommand.test.ts index 750081582d5..b2b4dbd5efb 100644 --- a/packages/cli/src/ui/commands/hooksCommand.test.ts +++ b/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'); + }); + }); }); diff --git a/packages/cli/src/ui/commands/hooksCommand.ts b/packages/cli/src/ui/commands/hooksCommand.ts index c0474a75611..748c477bae4 100644 --- a/packages/cli/src/ui/commands/hooksCommand.ts +++ b/packages/cli/src/ui/commands/hooksCommand.ts @@ -15,7 +15,9 @@ import { t } from '../../i18n/index.js'; import type { HookRegistryEntry, SessionHookEntry, + HookEventName, } from '@qwen-code/qwen-code-core'; +import { supportsMatchers } from '../components/hooks/constants.js'; /** * Format hook source for display @@ -71,7 +73,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 +91,99 @@ const listCommand: SlashCommand = { }; } - // Group hooks by event - const hooksByEvent = new Map< - string, - Array<{ hook: HookRegistryEntry | SessionHookEntry; isSession: boolean }> - >(); + const normalizeMatcher = (matcher: string | undefined): string => { + const trimmed = matcher?.trim(); + return trimmed ? trimmed : '*'; + }; - // Add config hooks - for (const hook of configHooks) { - const eventName = hook.eventName; - if (!hooksByEvent.has(eventName)) { - hooksByEvent.set(eventName, []); + interface FlattenedHook { + name: string; + source: string; + } + + const hooksByEvent = new Map>(); + + 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(); + hooksByEvent.set(eventName, matcherMap); } - hooksByEvent.get(eventName)!.push({ hook, isSession: false }); + let bucket = matcherMap.get(matcherKey); + if (!bucket) { + bucket = []; + matcherMap.set(matcherKey, bucket); + } + 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 +205,6 @@ export const hooksCommand: SlashCommand = { context: CommandContext, args: string, ): Promise => { - // In interactive mode, open the hooks dialog const executionMode = context.executionMode ?? 'interactive'; if (executionMode === 'interactive') { return { @@ -200,7 +213,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: '' }; }, diff --git a/packages/cli/src/ui/components/hooks/HandlerListBody.test.tsx b/packages/cli/src/ui/components/hooks/HandlerListBody.test.tsx new file mode 100644 index 00000000000..085c87558a0 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HandlerListBody.test.tsx @@ -0,0 +1,267 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render } from 'ink-testing-library'; +import { HooksConfigSource, HookType } from '@qwen-code/qwen-code-core'; +import { HandlerListBody } from './HandlerListBody.js'; +import type { HookConfigDisplayInfo } from './types.js'; + +vi.mock('../../../i18n/index.js', () => ({ + t: vi.fn((key: string) => key), +})); + +vi.mock('../../hooks/useTerminalSize.js', () => ({ + useTerminalSize: vi.fn(() => ({ columns: 120, rows: 24 })), +})); + +vi.mock('../../semantic-colors.js', () => ({ + theme: { + text: { primary: 'white', secondary: 'gray', accent: 'cyan' }, + }, +})); + +function commandConfig( + command = '/cmd.sh', + async = false, +): HookConfigDisplayInfo { + return { + config: { type: HookType.Command, command, async }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }; +} + +function httpConfig( + overrides: Partial<{ name: string; url: string }> = {}, +): HookConfigDisplayInfo { + return { + config: { + type: HookType.Http, + url: overrides.url ?? 'https://example.test/hook', + ...(overrides.name ? { name: overrides.name } : {}), + } as HookConfigDisplayInfo['config'], + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }; +} + +function functionConfig( + overrides: Partial<{ name: string; id: string }> = {}, +): HookConfigDisplayInfo { + return { + config: { + type: HookType.Function, + callback: async () => undefined, + errorMessage: 'fn failed', + id: overrides.id ?? 'fn-id', + ...(overrides.name ? { name: overrides.name } : {}), + } as HookConfigDisplayInfo['config'], + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }; +} + +function promptConfig( + overrides: Partial<{ name: string; prompt: string }> = {}, +): HookConfigDisplayInfo { + return { + config: { + type: HookType.Prompt, + prompt: overrides.prompt ?? 'short prompt', + ...(overrides.name ? { name: overrides.name } : {}), + } as HookConfigDisplayInfo['config'], + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }; +} + +describe('HandlerListBody', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('describeHook (rendered as the row label)', () => { + it('renders the command path for command hooks', () => { + const { lastFrame } = render( + , + ); + const out = lastFrame() ?? ''; + expect(out).toContain('[command]'); + expect(out).toContain('/check.sh'); + }); + + it('marks async command hooks with " async" in the type column', () => { + const { lastFrame } = render( + , + ); + expect(lastFrame() ?? '').toContain('[command async]'); + }); + + it('prefers the http hook name over the URL', () => { + const { lastFrame } = render( + , + ); + const out = lastFrame() ?? ''; + expect(out).toContain('[http]'); + expect(out).toContain('webhook-A'); + expect(out).not.toContain('https://x'); + }); + + it('falls back to the http URL when name is missing', () => { + const { lastFrame } = render( + , + ); + expect(lastFrame() ?? '').toContain('https://example.test/hook'); + }); + + it('prefers the function hook name over the id', () => { + const { lastFrame } = render( + , + ); + const out = lastFrame() ?? ''; + expect(out).toContain('[function]'); + expect(out).toContain('fn-name'); + expect(out).not.toContain('fn-id'); + }); + + it('falls back to function id, then to "function-hook" placeholder', () => { + const withId = render( + , + ); + expect(withId.lastFrame() ?? '').toContain('only-id'); + + const noNameNoId = render( + undefined, + errorMessage: 'fn failed', + } as HookConfigDisplayInfo['config'], + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }, + ]} + selectedIndex={0} + />, + ); + expect(noNameNoId.lastFrame() ?? '').toContain('function-hook'); + }); + + it('truncates long prompt text with "..." and keeps short prompts intact', () => { + const long = 'a'.repeat(80); + const { lastFrame: longFrame } = render( + , + ); + const longOut = longFrame() ?? ''; + expect(longOut).toContain('a'.repeat(50) + '...'); + expect(longOut).not.toContain('a'.repeat(51)); + + const { lastFrame: shortFrame } = render( + , + ); + const shortOut = shortFrame() ?? ''; + expect(shortOut).toContain('fits'); + expect(shortOut).not.toContain('...'); + }); + + it('prefers the prompt name over the prompt text', () => { + const { lastFrame } = render( + , + ); + const out = lastFrame() ?? ''; + expect(out).toContain('classifier'); + expect(out).not.toContain('should not appear'); + }); + }); + + describe('source column', () => { + it('appends the extension name for Extensions-source configs', () => { + const config: HookConfigDisplayInfo = { + config: { type: HookType.Command, command: '/ext.sh' }, + source: HooksConfigSource.Extensions, + sourceDisplay: 'my-extension', + enabled: true, + }; + + const { lastFrame } = render( + , + ); + + const out = lastFrame() ?? ''; + expect(out).toContain('Extensions'); + expect(out).toContain('my-extension'); + }); + + it('uses the long "Session (temporary)" label for session-source configs', () => { + const config: HookConfigDisplayInfo = { + config: { type: HookType.Command, command: '/sess.sh' }, + source: HooksConfigSource.Session, + sourceDisplay: 'Session (temporary)', + enabled: true, + }; + + const { lastFrame } = render( + , + ); + + expect(lastFrame() ?? '').toContain('Session (temporary)'); + }); + }); + + it('renders numbered rows and footer hint, places arrow on selected', () => { + const configs = [commandConfig('/first.sh'), commandConfig('/second.sh')]; + const { lastFrame } = render( + , + ); + const out = lastFrame() ?? ''; + + expect(out).toContain('1.'); + expect(out).toContain('2.'); + expect(out).toContain('Configured hooks:'); + expect(out).toContain('Enter to select · Esc to go back'); + + const arrowLine = out.split('\n').find((line) => line.includes('❯')); + expect(arrowLine).toBeDefined(); + expect(arrowLine).toContain('/second.sh'); + }); +}); diff --git a/packages/cli/src/ui/components/hooks/HandlerListBody.tsx b/packages/cli/src/ui/components/hooks/HandlerListBody.tsx new file mode 100644 index 00000000000..4c405077922 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HandlerListBody.tsx @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import { useTerminalSize } from '../../hooks/useTerminalSize.js'; +import { HookType } from '@qwen-code/qwen-code-core'; +import type { HookConfigDisplayInfo } from './types.js'; +import { getConfigSourceDisplay } from './sourceLabels.js'; +import { t } from '../../../i18n/index.js'; + +interface HandlerListBodyProps { + configs: HookConfigDisplayInfo[]; + selectedIndex: number; +} + +export function HandlerListBody({ + configs, + selectedIndex, +}: HandlerListBodyProps): React.JSX.Element { + const { columns: terminalWidth } = useTerminalSize(); + const commandWidth = Math.floor(terminalWidth * 0.65); + const sourceWidth = Math.floor(terminalWidth * 0.3); + + return ( + <> + + {t('Configured hooks:')} + + {configs.map((config, index) => { + const isSelected = index === selectedIndex; + const sourceDisplay = getConfigSourceDisplay(config); + const hookDisplay = describeHook(config); + const typeDisplay = formatTypeDisplay(config); + + return ( + + + + + {isSelected ? '❯' : ' '} + + + + {`${index + 1}. [${typeDisplay}] ${hookDisplay}`} + + + + + + {sourceDisplay} + + + + ); + })} + + + {t('Enter to select · Esc to go back')} + + + + ); +} + +function describeHook(info: HookConfigDisplayInfo): string { + const { config } = info; + switch (config.type) { + case HookType.Command: + return config.command || ''; + case HookType.Http: + return config.name || config.url || ''; + case HookType.Function: + return config.name || config.id || 'function-hook'; + case HookType.Prompt: { + const promptText = config.prompt || ''; + const maxLength = 50; + return ( + config.name || + (promptText.length > maxLength + ? promptText.slice(0, maxLength) + '...' + : promptText) + ); + } + default: { + const _exhaustive: never = config; + void _exhaustive; + return ''; + } + } +} + +function formatTypeDisplay(info: HookConfigDisplayInfo): string { + const { config } = info; + const isAsync = config.type === HookType.Command && config.async === true; + return isAsync ? `${config.type} async` : String(config.type); +} diff --git a/packages/cli/src/ui/components/hooks/HookConfigDetailStep.test.tsx b/packages/cli/src/ui/components/hooks/HookConfigDetailStep.test.tsx index 1f2728965aa..4175b7117be 100644 --- a/packages/cli/src/ui/components/hooks/HookConfigDetailStep.test.tsx +++ b/packages/cli/src/ui/components/hooks/HookConfigDetailStep.test.tsx @@ -14,17 +14,14 @@ import { import { HookConfigDetailStep } from './HookConfigDetailStep.js'; import type { HookEventDisplayInfo, HookConfigDisplayInfo } from './types.js'; -// Mock i18n module vi.mock('../../../i18n/index.js', () => ({ t: vi.fn((key: string) => key), })); -// Mock useTerminalSize vi.mock('../../hooks/useTerminalSize.js', () => ({ useTerminalSize: vi.fn(() => ({ columns: 100, rows: 24 })), })); -// Mock semantic-colors vi.mock('../../semantic-colors.js', () => ({ theme: { text: { @@ -51,7 +48,7 @@ describe('HookConfigDetailStep', () => { }, { code: 'Other', description: 'show stderr to user only' }, ], - configs: [], + matcherGroups: [], }); const createMockHookConfig = ( @@ -170,7 +167,6 @@ describe('HookConfigDetailStep', () => { , ); - // Should not have Extension label for User Settings const output = lastFrame(); const extensionMatch = output?.match(/Extension:/g); expect(extensionMatch).toBeNull(); @@ -252,6 +248,74 @@ describe('HookConfigDetailStep', () => { expect(lastFrame()).toContain('Esc to go back'); }); + it('should render Matcher field for matcher-capable events', () => { + const hookEvent = { + ...createMockHookEvent(), + event: HookEventName.PreToolUse, + }; + const hookConfig: HookConfigDisplayInfo = { + config: { + type: HookType.Command, + command: '/path/to/hook.sh', + }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + matcher: 'Bash', + enabled: true, + }; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Matcher:'); + expect(lastFrame()).toContain('Bash'); + }); + + it('should render Matcher as * when matcher is missing for matcher-capable events', () => { + const hookEvent = { + ...createMockHookEvent(), + event: HookEventName.PreToolUse, + }; + const hookConfig: HookConfigDisplayInfo = { + config: { + type: HookType.Command, + command: '/path/to/hook.sh', + }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Matcher:'); + expect(lastFrame()).toContain('*'); + }); + + it('should not render Matcher field for non-matcher events', () => { + const hookEvent = createMockHookEvent(); + const hookConfig: HookConfigDisplayInfo = { + config: { + type: HookType.Command, + command: '/path/to/hook.sh', + }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + matcher: '*', + enabled: true, + }; + + const output = + render( + , + ).lastFrame() ?? ''; + + expect(output).not.toContain('Matcher:'); + }); + it('should handle different event types', () => { const events = [ HookEventName.PreToolUse, @@ -266,7 +330,7 @@ describe('HookConfigDetailStep', () => { shortDescription: 'Test', description: '', exitCodes: [], - configs: [], + matcherGroups: [], }; const hookConfig = createMockHookConfig(); diff --git a/packages/cli/src/ui/components/hooks/HookConfigDetailStep.tsx b/packages/cli/src/ui/components/hooks/HookConfigDetailStep.tsx index 04aaad7cc69..ef5db0c4c40 100644 --- a/packages/cli/src/ui/components/hooks/HookConfigDetailStep.tsx +++ b/packages/cli/src/ui/components/hooks/HookConfigDetailStep.tsx @@ -10,6 +10,10 @@ import { useTerminalSize } from '../../hooks/useTerminalSize.js'; import type { HookConfigDisplayInfo, HookEventDisplayInfo } from './types.js'; import { HooksConfigSource } from '@qwen-code/qwen-code-core'; import { t } from '../../../i18n/index.js'; +import { + getTranslatedSourceDisplayMap, + supportsMatchers, +} from './constants.js'; interface HookConfigDetailStepProps { hookEvent: HookEventDisplayInfo; @@ -22,26 +26,10 @@ export function HookConfigDetailStep({ }: HookConfigDetailStepProps): React.JSX.Element { const { columns: terminalWidth } = useTerminalSize(); - // Get source display - const getSourceDisplay = (): string => { - switch (hookConfig.source) { - case HooksConfigSource.Project: - return t('Local Settings'); - case HooksConfigSource.User: - return t('User Settings'); - case HooksConfigSource.System: - return t('System Settings'); - case HooksConfigSource.Extensions: - return t('Extensions'); - default: - return hookConfig.source; - } - }; + const sourceDisplay = getTranslatedSourceDisplayMap()[hookConfig.source]; - // Check if this is from an extension const isFromExtension = hookConfig.source === HooksConfigSource.Extensions; - // Get hook type display const getHookTypeDisplay = (): string => { switch (hookConfig.config.type) { case 'command': @@ -51,7 +39,6 @@ export function HookConfigDetailStep({ } }; - // Get command to display const getCommand = (): string => { if (hookConfig.config.type === 'command') { return hookConfig.config.command; @@ -59,7 +46,6 @@ export function HookConfigDetailStep({ return ''; }; - // Get prompt to display const getPrompt = (): string => { if (hookConfig.config.type === 'prompt') { return hookConfig.config.prompt; @@ -67,7 +53,6 @@ export function HookConfigDetailStep({ return ''; }; - // Get URL to display const getUrl = (): string => { if (hookConfig.config.type === 'http') { return hookConfig.config.url; @@ -75,22 +60,19 @@ export function HookConfigDetailStep({ return ''; }; - // Calculate box width for command display const commandBoxWidth = Math.min(terminalWidth - 6, 80); - // Label width for alignment (Extension: is the longest label) const labelWidth = 12; + const showMatcher = supportsMatchers(hookEvent.event); return ( - {/* Title */} {t('Hook details')} - {/* Event */} {t('Event:')} @@ -98,7 +80,15 @@ export function HookConfigDetailStep({ {hookEvent.event} - {/* Type */} + {showMatcher && ( + + + {t('Matcher:')} + + {hookConfig.matcher || '*'} + + )} + {t('Type:')} @@ -106,18 +96,16 @@ export function HookConfigDetailStep({ {getHookTypeDisplay()} - {/* Source */} {t('Source:')} - {getSourceDisplay()} + {sourceDisplay} {hookConfig.sourcePath && ( ({hookConfig.sourcePath}) )} - {/* Extension name (only for extensions) */} {isFromExtension && hookConfig.sourceDisplay && ( @@ -127,7 +115,6 @@ export function HookConfigDetailStep({ )} - {/* Name (if exists) */} {hookConfig.config.name && ( @@ -137,7 +124,6 @@ export function HookConfigDetailStep({ )} - {/* Description (if exists) */} {hookConfig.config.description && ( @@ -149,7 +135,6 @@ export function HookConfigDetailStep({ )} - {/* Command / Prompt / URL - based on hook type */} {hookConfig.config.type === 'command' && ( <> @@ -201,7 +186,6 @@ export function HookConfigDetailStep({ )} - {/* Help text */} {t( @@ -210,7 +194,6 @@ export function HookConfigDetailStep({ - {/* Footer hint */} {t('Esc to go back')} diff --git a/packages/cli/src/ui/components/hooks/HookDetailStep.test.tsx b/packages/cli/src/ui/components/hooks/HookDetailStep.test.tsx index 0b5f1c6b7ee..f0371160e9d 100644 --- a/packages/cli/src/ui/components/hooks/HookDetailStep.test.tsx +++ b/packages/cli/src/ui/components/hooks/HookDetailStep.test.tsx @@ -12,19 +12,24 @@ import { HookType, } from '@qwen-code/qwen-code-core'; import { HookDetailStep } from './HookDetailStep.js'; -import type { HookEventDisplayInfo } from './types.js'; +import type { HookConfigDisplayInfo, HookEventDisplayInfo } from './types.js'; -// Mock i18n module vi.mock('../../../i18n/index.js', () => ({ - t: vi.fn((key: string) => key), + t: vi.fn((key: string, options?: { count?: string }) => { + if (key === '{{count}} hook' && options?.count) { + return `${options.count} hook`; + } + if (key === '{{count}} hooks' && options?.count) { + return `${options.count} hooks`; + } + return key; + }), })); -// Mock useTerminalSize vi.mock('../../hooks/useTerminalSize.js', () => ({ useTerminalSize: vi.fn(() => ({ columns: 100, rows: 24 })), })); -// Mock semantic-colors vi.mock('../../semantic-colors.js', () => ({ theme: { text: { @@ -39,190 +44,282 @@ vi.mock('../../semantic-colors.js', () => ({ }, })); -describe('HookDetailStep', () => { - const createMockHookInfo = ( - event: HookEventName, - configCount = 0, - hasDescription = true, - ): HookEventDisplayInfo => ({ - event, - shortDescription: `Short description for ${event}`, - description: hasDescription ? `Detailed description for ${event}` : '', - exitCodes: [ +function makeConfig( + command: string, + source: HooksConfigSource = HooksConfigSource.User, + matcher = '*', +): HookConfigDisplayInfo { + return { + config: { command, type: HookType.Command }, + source, + sourceDisplay: + source === HooksConfigSource.User ? 'User Settings' : 'Local Settings', + matcher, + enabled: true, + }; +} + +function makeHookInfo( + groups: Array<{ + matcher: string; + configs: HookConfigDisplayInfo[]; + sequential?: boolean; + }>, + opts: { + event?: HookEventName; + description?: string; + exitCodes?: HookEventDisplayInfo['exitCodes']; + flatConfigs?: HookConfigDisplayInfo[]; + } = {}, +): HookEventDisplayInfo { + const matcherGroups = opts.flatConfigs + ? [{ matcher: '*', configs: opts.flatConfigs, sequential: false }] + : groups; + return { + event: opts.event ?? HookEventName.PreToolUse, + shortDescription: 'short', + description: opts.description ?? '', + exitCodes: opts.exitCodes ?? [ { code: 0, description: 'Success' }, { code: 2, description: 'Block' }, ], - configs: Array(configCount) - .fill(null) - .map((_, i) => ({ - config: { command: `hook-command-${i}`, type: HookType.Command }, - source: - i % 2 === 0 ? HooksConfigSource.User : HooksConfigSource.Project, - sourceDisplay: i % 2 === 0 ? 'User Settings' : 'Local Settings', - enabled: true, - })), - }); + matcherGroups, + }; +} +describe('HookDetailStep', () => { beforeEach(() => { vi.clearAllMocks(); }); - it('should render hook event name as title', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse); + it('renders the "Event - Matchers" title', () => { + const hook = makeHookInfo([ + { matcher: '*', configs: [makeConfig('/x.sh')] }, + ]); const { lastFrame } = render( , ); - expect(lastFrame()).toContain(HookEventName.PreToolUse); + expect(lastFrame()).toContain(`${HookEventName.PreToolUse} - Matchers`); }); - it('should render description when present', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse, 0, true); - - const { lastFrame } = render( - , + it('renders event description when present', () => { + const hook = makeHookInfo( + [{ matcher: '*', configs: [makeConfig('/x.sh')] }], + { description: 'desc-for-event' }, ); - expect(lastFrame()).toContain('Detailed description for PreToolUse'); - }); - - it('should not render description section when empty', () => { - const hook = createMockHookInfo(HookEventName.Stop, 0, false); - const { lastFrame } = render( , ); - // Stop event has empty description - const output = lastFrame(); - expect(output).toContain(HookEventName.Stop); + expect(lastFrame()).toContain('desc-for-event'); }); - it('should render exit codes', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse); - - const { lastFrame } = render( - , + it('renders inline exit code descriptions', () => { + const hook = makeHookInfo([ + { matcher: '*', configs: [makeConfig('/x.sh')] }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('Exit code'); + expect(out).toContain('Success'); + expect(out).toContain('Block'); + const hook2 = makeHookInfo( + [{ matcher: '*', configs: [makeConfig('/x.sh')] }], + { + exitCodes: [{ code: 'Other', description: 'other-desc' }], + }, ); - - const output = lastFrame(); - expect(output).toContain('Exit codes'); - expect(output).toContain('0'); - expect(output).toContain('Success'); - expect(output).toContain('2'); - expect(output).toContain('Block'); + const out2 = + render().lastFrame() ?? + ''; + expect(out2).toContain('Other exit codes'); + expect(out2).toContain('other-desc'); }); - it('should show empty state when no configs', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse, 0); + it('shows empty state when no matcher groups', () => { + const hook = makeHookInfo([]); - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).toContain('No hooks configured for this event'); - expect(output).toContain('To add hooks, edit settings.json'); + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('No hooks configured for this event'); + expect(out).toContain('Esc to go back'); }); - it('should show configured hooks list when configs exist', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse, 2); - - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).toContain('Configured hooks'); - expect(output).toContain('[command]'); - expect(output).toContain('hook-command-0'); - expect(output).toContain('hook-command-1'); + it('renders matcher rows with [Source] label and matcher', () => { + const hook = makeHookInfo([ + { + matcher: '*', + configs: [makeConfig('/star.sh', HooksConfigSource.User, '*')], + }, + { + matcher: 'Bash', + configs: [makeConfig('/bash.sh', HooksConfigSource.User, 'Bash')], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('[User] *'); + expect(out).toContain('[User] Bash'); }); - it('should show source display for each config', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse, 2); - - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).toContain('User Settings'); - expect(output).toContain('Local Settings'); + it('uses Project label for workspace-source matcher groups', () => { + const hook = makeHookInfo([ + { + matcher: 'Bash', + configs: [makeConfig('/bash.sh', HooksConfigSource.Project, 'Bash')], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('[Project] Bash'); }); - it('should show selection indicator for first config', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse, 3); - - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).toContain('❯'); + it('renders all unique source labels for mixed-source matcher groups', () => { + const hook = makeHookInfo([ + { + matcher: 'Bash', + configs: [ + makeConfig('/user.sh', HooksConfigSource.User, 'Bash'), + makeConfig('/project.sh', HooksConfigSource.Project, 'Bash'), + makeConfig('/user-two.sh', HooksConfigSource.User, 'Bash'), + ], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('[User, Project] Bash'); }); - it('should show keyboard hint for going back', () => { - const hook = createMockHookInfo(HookEventName.PreToolUse); - - const { lastFrame } = render( - , - ); - - expect(lastFrame()).toContain('Esc to go back'); + it('renders singular "1 hook" and plural "N hooks"', () => { + const hook = makeHookInfo([ + { + matcher: '*', + configs: [makeConfig('/a.sh', HooksConfigSource.User, '*')], + }, + { + matcher: 'Bash', + configs: [ + makeConfig('/b.sh', HooksConfigSource.User, 'Bash'), + makeConfig('/c.sh', HooksConfigSource.User, 'Bash'), + ], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('1 hook'); + expect(out).toContain('2 hooks'); }); - it('should render with multiple configs', () => { - const hook = createMockHookInfo(HookEventName.PostToolUse, 5); - - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).toContain('1.'); - expect(output).toContain('2.'); - expect(output).toContain('3.'); - expect(output).toContain('4.'); - expect(output).toContain('5.'); + it('does not render specific command text on the matcher list page', () => { + const hook = makeHookInfo([ + { + matcher: 'Bash', + configs: [ + makeConfig( + '/very-specific-command.sh', + HooksConfigSource.User, + 'Bash', + ), + ], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + expect(out).not.toContain('/very-specific-command.sh'); }); - it('should handle hook with no exit codes', () => { - const hook: HookEventDisplayInfo = { - event: HookEventName.PreToolUse, - shortDescription: 'Test', - description: 'Test description', - exitCodes: [], - configs: [], - }; - - const { lastFrame } = render( - , - ); - - const output = lastFrame(); - expect(output).not.toContain('Exit codes'); + it('places the selection arrow on the selected matcher row', () => { + const hook = makeHookInfo([ + { + matcher: '*', + configs: [makeConfig('/a.sh', HooksConfigSource.User, '*')], + }, + { + matcher: 'Bash', + configs: [makeConfig('/b.sh', HooksConfigSource.User, 'Bash')], + }, + ]); + + const out = + render().lastFrame() ?? + ''; + const arrowLine = out.split('\n').find((line) => line.includes('❯')); + expect(arrowLine).toBeDefined(); + expect(arrowLine).toContain('Bash'); }); - it('should handle different hook event types', () => { - const events = [ - HookEventName.Stop, - HookEventName.PreToolUse, - HookEventName.PostToolUse, - HookEventName.UserPromptSubmit, - HookEventName.SessionStart, - HookEventName.SessionEnd, - ]; + it('renders the Enter/Esc footer hint', () => { + const hook = makeHookInfo([ + { matcher: '*', configs: [makeConfig('/x.sh')] }, + ]); - for (const event of events) { - const hook = createMockHookInfo(event, 1); - - const { lastFrame } = render( - , - ); + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('Enter to select'); + expect(out).toContain('Esc to go back'); + }); - expect(lastFrame()).toContain(event); - } + describe('non-matcher events (e.g. Stop)', () => { + it('does not append " - Matchers" to the title', () => { + const hook = makeHookInfo([], { + event: HookEventName.Stop, + flatConfigs: [makeConfig('/stop.sh', HooksConfigSource.User, '*')], + }); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain(HookEventName.Stop); + expect(out).not.toContain('- Matchers'); + }); + + it('renders the handler list directly (with command and source)', () => { + const hook = makeHookInfo([], { + event: HookEventName.Stop, + flatConfigs: [ + makeConfig('/stop-one.sh', HooksConfigSource.User, '*'), + makeConfig('/stop-two.sh', HooksConfigSource.Project, '*'), + ], + }); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('[command]'); + expect(out).toContain('/stop-one.sh'); + expect(out).toContain('/stop-two.sh'); + expect(out).toContain('User Settings'); + expect(out).toContain('Local Settings'); + expect(out).toContain('Enter to select'); + }); + + it('shows empty state when a non-matcher event has no handlers', () => { + const hook = makeHookInfo([], { + event: HookEventName.Stop, + flatConfigs: [], + }); + + const out = + render().lastFrame() ?? + ''; + expect(out).toContain('No hooks configured for this event'); + }); }); }); diff --git a/packages/cli/src/ui/components/hooks/HookDetailStep.tsx b/packages/cli/src/ui/components/hooks/HookDetailStep.tsx index 9dd691ef679..08da705da18 100644 --- a/packages/cli/src/ui/components/hooks/HookDetailStep.tsx +++ b/packages/cli/src/ui/components/hooks/HookDetailStep.tsx @@ -4,13 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Box, Text } from 'ink'; -import { theme } from '../../semantic-colors.js'; -import { useTerminalSize } from '../../hooks/useTerminalSize.js'; import type { HookEventDisplayInfo } from './types.js'; -import { HooksConfigSource, HookType } from '@qwen-code/qwen-code-core'; -import { getTranslatedSourceDisplayMap } from './constants.js'; -import { t } from '../../../i18n/index.js'; +import { supportsMatchers } from './constants.js'; +import { HookEventMatcherListStep } from './HookEventMatcherListStep.js'; +import { HookEventHandlerListStep } from './HookEventHandlerListStep.js'; interface HookDetailStepProps { hook: HookEventDisplayInfo; @@ -21,159 +18,10 @@ export function HookDetailStep({ hook, selectedIndex, }: HookDetailStepProps): React.JSX.Element { - const hasConfigs = hook.configs.length > 0; - const { columns: terminalWidth } = useTerminalSize(); - - // Get translated source display map - const sourceDisplayMap = getTranslatedSourceDisplayMap(); - - // Calculate column widths (command: 70%, source: 30%) - const commandWidth = Math.floor(terminalWidth * 0.65); - const sourceWidth = Math.floor(terminalWidth * 0.3); - - // Get source display for config list - const getConfigSourceDisplay = (config: { - source: HooksConfigSource; - sourceDisplay: string; - }): string => { - if (config.source === HooksConfigSource.Extensions) { - // For extensions, sourceDisplay is the extension name - return `${sourceDisplayMap[HooksConfigSource.Extensions]} (${config.sourceDisplay})`; - } - return sourceDisplayMap[config.source] || config.source; - }; - - return ( - - {/* Title */} - - - {hook.event} - - - - {/* Description */} - {hook.description && ( - - {hook.description} - - )} - - {/* Exit codes */} - {hook.exitCodes.length > 0 && ( - - - {t('Exit codes:')} - - {hook.exitCodes.map((ec, index) => ( - - - {` ${ec.code}: ${ec.description}`} - - - ))} - - )} - - - - {/* Configs or empty state */} - {hasConfigs ? ( - <> - - {t('Configured hooks:')} - - {hook.configs.map((config, index) => { - const isSelected = index === selectedIndex; - const sourceDisplay = getConfigSourceDisplay(config); - - // Get display text based on hook type - let hookDisplay = ''; - const hookType = config.config.type; - - if (hookType === HookType.Command) { - // For command hook, show command (truncate if too long) - hookDisplay = config.config.command || ''; - } else if (hookType === HookType.Http) { - // For http hook, show name or url - hookDisplay = config.config.name || config.config.url || ''; - } else if (hookType === HookType.Function) { - // For function hook, show name or id - hookDisplay = - config.config.name || config.config.id || 'function-hook'; - } else if (hookType === HookType.Prompt) { - // For prompt hook, show name or prompt content (truncated) - const promptText = config.config.prompt || ''; - const maxLength = 50; - hookDisplay = - config.config.name || - (promptText.length > maxLength - ? promptText.slice(0, maxLength) + '...' - : promptText); - } - - // Check if this is an async hook (only command hooks support async) - const isAsync = - hookType === HookType.Command && config.config.async === true; - const typeDisplay = isAsync - ? `${hookType} async` - : String(hookType); - - return ( - - {/* Left column: selector + display */} - - - - {isSelected ? '❯' : ' '} - - - - {`${index + 1}. [${typeDisplay}] ${hookDisplay}`} - - - {/* Spacer between columns */} - - {/* Right column: source */} - - - {sourceDisplay} - - - - ); - })} - - - {t('Enter to select · Esc to go back')} - - - - ) : ( - <> - - - {t('No hooks configured for this event.')} - - - - - {t('To add hooks, edit settings.json directly or ask Qwen.')} - - - - {t('Esc to go back')} - - - )} - - ); + if (supportsMatchers(hook.event)) { + return ( + + ); + } + return ; } diff --git a/packages/cli/src/ui/components/hooks/HookEventHandlerListStep.tsx b/packages/cli/src/ui/components/hooks/HookEventHandlerListStep.tsx new file mode 100644 index 00000000000..f88bf5376db --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HookEventHandlerListStep.tsx @@ -0,0 +1,56 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import type { HookEventDisplayInfo } from './types.js'; +import { HookEventHeader } from './HookEventHeader.js'; +import { HandlerListBody } from './HandlerListBody.js'; +import { getAllConfigs } from './matcherGrouping.js'; +import { t } from '../../../i18n/index.js'; + +interface HookEventHandlerListStepProps { + hook: HookEventDisplayInfo; + selectedIndex: number; +} + +export function HookEventHandlerListStep({ + hook, + selectedIndex, +}: HookEventHandlerListStepProps): React.JSX.Element { + const flatConfigs = getAllConfigs(hook); + const hasConfigs = flatConfigs.length > 0; + + return ( + + + + {hasConfigs ? ( + + ) : ( + <> + + + {t('No hooks configured for this event.')} + + + + + {t('To add hooks, edit settings.json directly or ask Qwen.')} + + + + {t('Esc to go back')} + + + )} + + ); +} diff --git a/packages/cli/src/ui/components/hooks/HookEventHeader.tsx b/packages/cli/src/ui/components/hooks/HookEventHeader.tsx new file mode 100644 index 00000000000..7bbd829e39e --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HookEventHeader.tsx @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import type { HookEventDisplayInfo } from './types.js'; +import { t } from '../../../i18n/index.js'; + +interface HookEventHeaderProps { + title: string; + description: string; + exitCodes: HookEventDisplayInfo['exitCodes']; +} + +export function HookEventHeader({ + title, + description, + exitCodes, +}: HookEventHeaderProps): React.JSX.Element { + return ( + <> + + + {title} + + + + {description && ( + + {description} + + )} + + + + ); +} + +function ExitCodesBlock({ + exitCodes, +}: { + exitCodes: HookEventDisplayInfo['exitCodes']; +}): React.JSX.Element | null { + if (exitCodes.length === 0) return null; + return ( + + {exitCodes.map((ec, index) => { + const label = + typeof ec.code === 'number' + ? `${t('Exit code')} ${ec.code}` + : `${t('Other exit codes')}`; + return ( + + + {label} - {ec.description} + + + ); + })} + + ); +} diff --git a/packages/cli/src/ui/components/hooks/HookEventMatcherListStep.tsx b/packages/cli/src/ui/components/hooks/HookEventMatcherListStep.tsx new file mode 100644 index 00000000000..0e95fb0d528 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HookEventMatcherListStep.tsx @@ -0,0 +1,95 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import { useTerminalSize } from '../../hooks/useTerminalSize.js'; +import type { HookEventDisplayInfo } from './types.js'; +import { HookEventHeader } from './HookEventHeader.js'; +import { formatSourceLabels } from './sourceLabels.js'; +import { t } from '../../../i18n/index.js'; + +interface HookEventMatcherListStepProps { + hook: HookEventDisplayInfo; + selectedIndex: number; +} + +export function HookEventMatcherListStep({ + hook, + selectedIndex, +}: HookEventMatcherListStepProps): React.JSX.Element { + const { columns: terminalWidth } = useTerminalSize(); + const leftWidth = Math.floor(terminalWidth * 0.6); + const hasMatchers = hook.matcherGroups.length > 0; + + return ( + + + + {hasMatchers ? ( + <> + {hook.matcherGroups.map((group, index) => { + const isSelected = index === selectedIndex; + const sourceLabel = formatSourceLabels(group.configs); + const count = group.configs.length; + const countLabel = + count === 1 + ? t('{{count}} hook', { count: String(count) }) + : t('{{count}} hooks', { count: String(count) }); + const rowText = `${index + 1}. [${sourceLabel}] ${group.matcher}`; + + return ( + + + + {isSelected ? '❯' : ' '} + + + + + {rowText} + + + {countLabel} + + ); + })} + + + {t('Enter to select · Esc to go back')} + + + + ) : ( + <> + + + {t('No hooks configured for this event.')} + + + + + {t('To add hooks, edit settings.json directly or ask Qwen.')} + + + + {t('Esc to go back')} + + + )} + + ); +} diff --git a/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.test.tsx b/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.test.tsx new file mode 100644 index 00000000000..7002e8670be --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.test.tsx @@ -0,0 +1,252 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render } from 'ink-testing-library'; +import { + HookEventName, + HooksConfigSource, + HookType, +} from '@qwen-code/qwen-code-core'; +import { HookMatcherDetailStep } from './HookMatcherDetailStep.js'; +import type { + HookConfigDisplayInfo, + HookEventDisplayInfo, + HookMatcherDisplayInfo, +} from './types.js'; + +vi.mock('../../../i18n/index.js', () => ({ + t: vi.fn((key: string) => key), +})); + +vi.mock('../../hooks/useTerminalSize.js', () => ({ + useTerminalSize: vi.fn(() => ({ columns: 100, rows: 24 })), +})); + +vi.mock('../../semantic-colors.js', () => ({ + theme: { + text: { + primary: 'white', + secondary: 'gray', + accent: 'cyan', + }, + status: { + success: 'green', + error: 'red', + }, + }, +})); + +function makeEvent( + overrides: Partial = {}, +): HookEventDisplayInfo { + return { + event: HookEventName.PreToolUse, + shortDescription: 'short', + description: 'Input to command is JSON of tool call arguments.', + exitCodes: [ + { code: 0, description: 'stdout/stderr not shown' }, + { code: 2, description: 'show stderr to model and block tool call' }, + { + code: 'Other', + description: 'show stderr to user only but continue with tool call', + }, + ], + matcherGroups: [], + ...overrides, + }; +} + +function makeConfig( + command: string, + source: HooksConfigSource = HooksConfigSource.User, +): HookConfigDisplayInfo { + return { + config: { command, type: HookType.Command }, + source, + sourceDisplay: + source === HooksConfigSource.User ? 'User Settings' : 'Local Settings', + matcher: 'Bash', + enabled: true, + }; +} + +describe('HookMatcherDetailStep', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders the "Event - Matcher: " title', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [makeConfig('/x.sh')], + }; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain( + `${HookEventName.PreToolUse} - Matcher: Bash`, + ); + }); + + it('keeps the "Matcher:" prefix when the matcher is the * fallback', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: '*', + configs: [makeConfig('/x.sh')], + }; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain(`${HookEventName.PreToolUse} - Matcher: *`); + }); + + it('keeps the event description visible on the matcher detail page', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [makeConfig('/x.sh')], + }; + + const out = + render( + , + ).lastFrame() ?? ''; + + expect(out).toContain('Input to command is JSON of tool call arguments.'); + }); + + it('renders inline exit code descriptions on the matcher detail page', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [makeConfig('/x.sh')], + }; + + const out = + render( + , + ).lastFrame() ?? ''; + + expect(out).toContain('Exit code 0'); + expect(out).toContain('stdout/stderr not shown'); + expect(out).toContain('Exit code 2'); + expect(out).toContain('show stderr to model and block tool call'); + expect(out).toContain('Other exit codes'); + expect(out).toContain( + 'show stderr to user only but continue with tool call', + ); + }); + + it('renders the handler command for command hooks', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [makeConfig('/check.sh')], + }; + + const { lastFrame } = render( + , + ); + + const out = lastFrame() ?? ''; + expect(out).toContain('[command]'); + expect(out).toContain('/check.sh'); + }); + + it('renders multiple handler rows with numbering', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Edit|Write', + configs: [ + makeConfig('/first.sh'), + makeConfig('/second.sh', HooksConfigSource.Project), + ], + }; + + const { lastFrame } = render( + , + ); + + const out = lastFrame() ?? ''; + expect(out).toContain('1.'); + expect(out).toContain('2.'); + expect(out).toContain('/first.sh'); + expect(out).toContain('/second.sh'); + expect(out).toContain('User Settings'); + expect(out).toContain('Local Settings'); + }); + + it('places the selection arrow on the selected handler row', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [makeConfig('/first.sh'), makeConfig('/second.sh')], + }; + + const { lastFrame } = render( + , + ); + + const out = lastFrame() ?? ''; + const arrowLine = out.split('\n').find((line) => line.includes('❯')); + expect(arrowLine).toBeDefined(); + expect(arrowLine).toContain('/second.sh'); + }); + + it('renders empty state when the matcher group has no handlers', () => { + const hookEvent = makeEvent(); + const matcherGroup: HookMatcherDisplayInfo = { + matcher: 'Bash', + configs: [], + }; + + const { lastFrame } = render( + , + ); + + const out = lastFrame() ?? ''; + expect(out).toContain('No hooks configured for this matcher'); + expect(out).toContain('Esc to go back'); + }); +}); diff --git a/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.tsx b/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.tsx new file mode 100644 index 00000000000..06b2c05449b --- /dev/null +++ b/packages/cli/src/ui/components/hooks/HookMatcherDetailStep.tsx @@ -0,0 +1,54 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import type { HookEventDisplayInfo, HookMatcherDisplayInfo } from './types.js'; +import { HookEventHeader } from './HookEventHeader.js'; +import { HandlerListBody } from './HandlerListBody.js'; +import { t } from '../../../i18n/index.js'; + +interface HookMatcherDetailStepProps { + hookEvent: HookEventDisplayInfo; + matcherGroup: HookMatcherDisplayInfo; + selectedIndex: number; +} + +export function HookMatcherDetailStep({ + hookEvent, + matcherGroup, + selectedIndex, +}: HookMatcherDetailStepProps): React.JSX.Element { + const hasConfigs = matcherGroup.configs.length > 0; + + return ( + + + + {hasConfigs ? ( + + ) : ( + <> + + + {t('No hooks configured for this matcher.')} + + + + {t('Esc to go back')} + + + )} + + ); +} diff --git a/packages/cli/src/ui/components/hooks/HooksListStep.test.tsx b/packages/cli/src/ui/components/hooks/HooksListStep.test.tsx index a328ca66a05..c477247a261 100644 --- a/packages/cli/src/ui/components/hooks/HooksListStep.test.tsx +++ b/packages/cli/src/ui/components/hooks/HooksListStep.test.tsx @@ -14,10 +14,8 @@ import { import { HooksListStep } from './HooksListStep.js'; import type { HookEventDisplayInfo } from './types.js'; -// Mock i18n module vi.mock('../../../i18n/index.js', () => ({ t: vi.fn((key: string, options?: { count?: string }) => { - // Handle pluralization if (key === '{{count}} hook configured' && options?.count) { return `${options.count} hook configured`; } @@ -28,12 +26,10 @@ vi.mock('../../../i18n/index.js', () => ({ }), })); -// Mock useTerminalSize vi.mock('../../hooks/useTerminalSize.js', () => ({ useTerminalSize: vi.fn(() => ({ columns: 120, rows: 24 })), })); -// Mock semantic-colors vi.mock('../../semantic-colors.js', () => ({ theme: { text: { @@ -52,23 +48,30 @@ describe('HooksListStep', () => { const createMockHookInfo = ( event: HookEventName, configCount = 0, - ): HookEventDisplayInfo => ({ - event, - shortDescription: `Description for ${event}`, - description: `Detailed description for ${event}`, - exitCodes: [ - { code: 0, description: 'Success' }, - { code: 2, description: 'Block' }, - ], - configs: Array(configCount) + ): HookEventDisplayInfo => { + const configs = Array(configCount) .fill(null) .map((_, i) => ({ - config: { command: `hook-${i}`, type: HookType.Command }, + config: { + command: `hook-${i}`, + type: HookType.Command as const, + }, source: HooksConfigSource.User, sourceDisplay: 'User Settings', + matcher: '*', enabled: true, - })), - }); + })); + return { + event, + shortDescription: `Description for ${event}`, + description: `Detailed description for ${event}`, + exitCodes: [ + { code: 0, description: 'Success' }, + { code: 2, description: 'Block' }, + ], + matcherGroups: configs.length > 0 ? [{ matcher: '*', configs }] : [], + }; + }; beforeEach(() => { vi.clearAllMocks(); diff --git a/packages/cli/src/ui/components/hooks/HooksListStep.tsx b/packages/cli/src/ui/components/hooks/HooksListStep.tsx index 5b3da41f574..88db77485a0 100644 --- a/packages/cli/src/ui/components/hooks/HooksListStep.tsx +++ b/packages/cli/src/ui/components/hooks/HooksListStep.tsx @@ -10,6 +10,13 @@ import { useTerminalSize } from '../../hooks/useTerminalSize.js'; import type { HookEventDisplayInfo } from './types.js'; import { t } from '../../../i18n/index.js'; +function configCountFor(hook: HookEventDisplayInfo): number { + return hook.matcherGroups.reduce( + (sum, group) => sum + group.configs.length, + 0, + ); +} + interface HooksListStepProps { hooks: HookEventDisplayInfo[]; selectedIndex: number; @@ -21,7 +28,6 @@ export function HooksListStep({ }: HooksListStepProps): React.JSX.Element { const { columns: terminalWidth } = useTerminalSize(); - // Calculate responsive width for hook name column (min 20, max 35) const hookNameWidth = Math.min( 35, Math.max(20, Math.floor(terminalWidth * 0.25)), @@ -35,13 +41,11 @@ export function HooksListStep({ ); } - // Calculate total configured hooks const totalConfigured = hooks.reduce( - (sum, hook) => sum + hook.configs.length, + (sum, hook) => sum + configCountFor(hook), 0, ); - // Get the correct plural/singular form const hooksConfiguredText = totalConfigured === 1 ? t('{{count}} hook configured', { count: String(totalConfigured) }) @@ -66,7 +70,7 @@ export function HooksListStep({ {hooks.map((hook, index) => { const isSelected = index === selectedIndex; - const configCount = hook.configs.length; + const configCount = configCountFor(hook); const maxDigits = String(hooks.length).length; const paddedIndex = String(index + 1).padStart(maxDigits); diff --git a/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx b/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx index 53330ffd244..321596b4b5b 100644 --- a/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx +++ b/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx @@ -8,19 +8,39 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { HooksManagementDialog } from './HooksManagementDialog.js'; import { renderWithProviders } from '../../../test-utils/render.js'; import { useKeypress } from '../../hooks/useKeypress.js'; +import { useConfig } from '../../contexts/ConfigContext.js'; import type { Key } from '../../contexts/KeypressContext.js'; -// Mock useKeypress vi.mock('../../hooks/useKeypress.js', () => ({ useKeypress: vi.fn(), })); const mockedUseKeypress = vi.mocked(useKeypress); +const mockedUseConfig = vi.mocked(useConfig); + +/** + * Returns a `useConfig` return value with `disableAllHooks` flipped on, while + * keeping every other method shaped like the default mock at the top of this + * file. Used with `mockReturnValueOnce` for the initial render — the dialog's + * navigation stack is seeded in a `useState` initializer that only consults + * `disableAllHooks` once, so subsequent renders falling back to the default + * mock is fine. + */ +function disabledHooksConfig(): ReturnType { + return { + getExtensions: vi.fn(() => []), + getDisableAllHooks: vi.fn(() => true), + getHookSystem: vi.fn(() => ({ + getSessionHooksManager: vi.fn(() => ({ + getAllSessionHooks: vi.fn(() => []), + })), + })), + getSessionId: vi.fn(() => 'test-session-id'), + } as unknown as ReturnType; +} -// Mock i18n module vi.mock('../../../i18n/index.js', () => ({ t: vi.fn((key: string, options?: { count?: string }) => { - // Handle pluralization if (key === '{{count}} hook configured' && options?.count) { return `${options.count} hook configured`; } @@ -33,7 +53,6 @@ vi.mock('../../../i18n/index.js', () => ({ if (key === '{{count}} configured hooks' && options?.count) { return `${options.count} configured hooks`; } - // Handle interpolation for disabled message if ( key === 'All hooks are currently disabled. You have {{count}} that are not running.' && @@ -45,12 +64,10 @@ vi.mock('../../../i18n/index.js', () => ({ }), })); -// Mock useTerminalSize vi.mock('../../hooks/useTerminalSize.js', () => ({ useTerminalSize: vi.fn(() => ({ columns: 120, rows: 24 })), })); -// Mock useConfig vi.mock('../../contexts/ConfigContext.js', async (importOriginal) => { const actual = await importOriginal(); @@ -69,7 +86,6 @@ vi.mock('../../contexts/ConfigContext.js', async (importOriginal) => { }; }); -// Mock loadSettings vi.mock('../../../config/settings.js', async (importOriginal) => { const actual = await importOriginal(); @@ -81,7 +97,6 @@ vi.mock('../../../config/settings.js', async (importOriginal) => { }; }); -// Mock semantic-colors vi.mock('../../semantic-colors.js', () => ({ theme: { text: { @@ -100,7 +115,6 @@ vi.mock('../../semantic-colors.js', () => ({ }, })); -// Mock createDebugLogger vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { const actual = await importOriginal(); @@ -108,12 +122,13 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { ...actual, createDebugLogger: vi.fn(() => ({ log: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), error: vi.fn(), })), }; }); -// Helper to create a key object function createKey(name: string, sequence = ''): Key { return { name, @@ -133,7 +148,6 @@ describe('HooksManagementDialog', () => { vi.clearAllMocks(); keypressHandler = null; - // Mock useKeypress to capture the handler mockedUseKeypress.mockImplementation((handler) => { keypressHandler = handler; }); @@ -143,117 +157,51 @@ describe('HooksManagementDialog', () => { keypressHandler = null; }); - describe('Initial rendering', () => { - it('should render loading state initially', () => { - const { lastFrame } = renderWithProviders( - , - ); - - expect(lastFrame()).toContain('Loading hooks'); - }); + it('should render loading state initially', () => { + const { lastFrame } = renderWithProviders( + , + ); - it('should render with border', async () => { - const { lastFrame, unmount } = renderWithProviders( - , - ); - - await new Promise((resolve) => setTimeout(resolve, 100)); - - // The dialog should have a border (rendered as box-drawing characters) - const output = lastFrame(); - expect(output).toBeTruthy(); - - unmount(); - }); + expect(lastFrame()).toContain('Loading hooks'); }); - describe('Keyboard navigation - HOOKS_LIST step', () => { - it('should register keypress handler with isActive: true', async () => { - renderWithProviders(); - - await new Promise((resolve) => setTimeout(resolve, 100)); - - expect(mockedUseKeypress).toHaveBeenCalled(); - const options = mockedUseKeypress.mock.calls[0][1]; - expect(options).toEqual({ isActive: true }); - }); - - it('should close dialog on Escape key', async () => { - renderWithProviders(); + it('should allow Escape to close during loading state', () => { + renderWithProviders(); - await new Promise((resolve) => setTimeout(resolve, 100)); + expect(keypressHandler).not.toBeNull(); + keypressHandler!(createKey('escape', '\x1b')); - expect(keypressHandler).not.toBeNull(); - keypressHandler!(createKey('escape', '\x1b')); - - expect(mockOnClose).toHaveBeenCalledTimes(1); - }); - - it('should not go above first item when pressing up', async () => { - const { unmount } = renderWithProviders( - , - ); - - await new Promise((resolve) => setTimeout(resolve, 100)); - - // Press up multiple times from first item - keypressHandler!(createKey('up')); - keypressHandler!(createKey('up')); - keypressHandler!(createKey('up')); - - // Should still be at first item (no crash) - unmount(); - }); + expect(mockOnClose).toHaveBeenCalledTimes(1); }); - describe('Keyboard navigation - HOOKS_DISABLED step', () => { - it('should show disabled state when disableAllHooks is true', async () => { - // Override the mock for this test - const configContext = await import('../../contexts/ConfigContext.js'); - vi.mocked(configContext.useConfig).mockReturnValue({ - getExtensions: vi.fn(() => []), - getDisableAllHooks: vi.fn(() => true), - } as unknown as ReturnType); - - const { lastFrame, unmount } = renderWithProviders( - , - ); - - await new Promise((resolve) => setTimeout(resolve, 100)); + it('should register the keypress handler with isActive: true', () => { + renderWithProviders(); - const output = lastFrame(); - expect(output).toContain('Hook Configuration - Disabled'); - - unmount(); - }); - - it('should close dialog on Escape key when hooks are disabled', async () => { - const configContext = await import('../../contexts/ConfigContext.js'); - vi.mocked(configContext.useConfig).mockReturnValue({ - getExtensions: vi.fn(() => []), - getDisableAllHooks: vi.fn(() => true), - } as unknown as ReturnType); - - renderWithProviders(); + expect(mockedUseKeypress).toHaveBeenCalled(); + expect(mockedUseKeypress.mock.calls[0][1]).toEqual({ isActive: true }); + }); - await new Promise((resolve) => setTimeout(resolve, 100)); + it('should render HOOKS_DISABLED step on first render when disableAllHooks is true', () => { + // `renderContent` checks the HOOKS_DISABLED branch before the isLoading + // branch, so the disabled view is visible synchronously on the initial + // render — no need to wait for the hooks-loading effect. + mockedUseConfig.mockReturnValueOnce(disabledHooksConfig()); - expect(keypressHandler).not.toBeNull(); - keypressHandler!(createKey('escape', '\x1b')); + const { lastFrame } = renderWithProviders( + , + ); - expect(mockOnClose).toHaveBeenCalledTimes(1); - }); + expect(lastFrame()).toContain('Hook Configuration - Disabled'); }); - describe('Loading and error states', () => { - it('should allow Escape to close during loading state', () => { - renderWithProviders(); + it('should close dialog on Escape when disableAllHooks is true', () => { + mockedUseConfig.mockReturnValueOnce(disabledHooksConfig()); - // Don't wait for loading to complete - expect(keypressHandler).not.toBeNull(); - keypressHandler!(createKey('escape', '\x1b')); + renderWithProviders(); - expect(mockOnClose).toHaveBeenCalledTimes(1); - }); + expect(keypressHandler).not.toBeNull(); + keypressHandler!(createKey('escape', '\x1b')); + + expect(mockOnClose).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx b/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx index d0ff2b33e87..6026784bf7d 100644 --- a/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx +++ b/packages/cli/src/ui/components/hooks/HooksManagementDialog.tsx @@ -25,28 +25,27 @@ import type { HookEventDisplayInfo, } from './types.js'; import { HOOKS_MANAGEMENT_STEPS } from './types.js'; +import { addConfigToMatcherGroup, getAllConfigs } from './matcherGrouping.js'; import { HooksListStep } from './HooksListStep.js'; import { HookDetailStep } from './HookDetailStep.js'; +import { HookMatcherDetailStep } from './HookMatcherDetailStep.js'; import { HookConfigDetailStep } from './HookConfigDetailStep.js'; import { HooksDisabledStep } from './HooksDisabledStep.js'; import { DISPLAY_HOOK_EVENTS, getTranslatedSourceDisplayMap, createEmptyHookEventInfo, + supportsMatchers, } from './constants.js'; import { t } from '../../../i18n/index.js'; const debugLogger = createDebugLogger('HOOKS_DIALOG'); -/** - * Type guard to check if a value is a valid HookConfig - */ function isValidHookConfig(config: unknown): config is HookConfig { if (typeof config !== 'object' || config === null || !('type' in config)) { return false; } const obj = config as Record; - // Check based on type if (obj['type'] === 'command') { return 'command' in obj && typeof obj['command'] === 'string'; } @@ -62,52 +61,37 @@ function isValidHookConfig(config: unknown): config is HookConfig { return false; } -/** - * Type guard to check if a value is a valid HookDefinition - */ function isValidHookDefinition(def: unknown): def is HookDefinition { if (typeof def !== 'object' || def === null) { return false; } const obj = def as Record; - // hooks array is required if (!('hooks' in obj) || !Array.isArray(obj['hooks'])) { return false; } - // Validate each hook config in the array for (const hook of obj['hooks']) { if (!isValidHookConfig(hook)) { return false; } } - // matcher is optional but must be a string if present if ('matcher' in obj && typeof obj['matcher'] !== 'string') { return false; } - // sequential is optional but must be a boolean if present if ('sequential' in obj && typeof obj['sequential'] !== 'boolean') { return false; } return true; } -/** - * Type guard to check if a value is a valid hooks record - * Note: This validates the structure but allows individual events to have - * invalid configs - those will be filtered out during processing. - */ function isValidHooksRecord(hooks: unknown): hooks is Record { if (typeof hooks !== 'object' || hooks === null) { return false; } - // Basic structure check - must be a record with array values for event keys const record = hooks as Record; for (const [key, value] of Object.entries(record)) { - // Skip non-event configuration fields if (HOOKS_CONFIG_FIELDS.includes(key)) { continue; } - // Event values should be arrays (even if contents are invalid) if (!Array.isArray(value)) { return false; } @@ -115,10 +99,6 @@ function isValidHooksRecord(hooks: unknown): hooks is Record { return true; } -/** - * Safely extract hook definitions for a specific event - * Returns empty array if the definitions are invalid - */ function getValidHookDefinitions( hooksRecord: Record, eventName: string, @@ -148,11 +128,6 @@ export function HooksManagementDialog({ const { columns: width } = useTerminalSize(); const boxWidth = width - 4; - // Check if hooks are disabled - // Note: This value is captured at dialog open time. If disableAllHooks - // changes while the dialog is open (e.g., via settings.json edit), - // the dialog will not react to the change until it's closed and reopened. - // This is intentional - the dialog represents a snapshot of the current state. const disableAllHooks = config?.getDisableAllHooks() ?? false; const [navigationStack, setNavigationStack] = useState([ @@ -161,20 +136,19 @@ export function HooksManagementDialog({ : HOOKS_MANAGEMENT_STEPS.HOOKS_LIST, ]); const [selectedHookIndex, setSelectedHookIndex] = useState(-1); + const [selectedMatcherIndex, setSelectedMatcherIndex] = useState(-1); const [selectedConfigIndex, setSelectedConfigIndex] = useState(-1); - // Track selected index within each step for keyboard navigation const [listSelectedIndex, setListSelectedIndex] = useState(0); const [detailSelectedIndex, setDetailSelectedIndex] = useState(0); + const [matcherSelectedIndex, setMatcherSelectedIndex] = useState(0); const [hooks, setHooks] = useState([]); const [isLoading, setIsLoading] = useState(true); const [loadError, setLoadError] = useState(null); - // Current step const currentStep = navigationStack[navigationStack.length - 1] || HOOKS_MANAGEMENT_STEPS.HOOKS_LIST; - // Selected hook event const selectedHook = useMemo(() => { if (selectedHookIndex >= 0 && selectedHookIndex < hooks.length) { return hooks[selectedHookIndex]; @@ -182,11 +156,20 @@ export function HooksManagementDialog({ return null; }, [hooks, selectedHookIndex]); - // Centralized keyboard handler + const selectedMatcher = useMemo(() => { + if ( + selectedHook && + selectedMatcherIndex >= 0 && + selectedMatcherIndex < selectedHook.matcherGroups.length + ) { + return selectedHook.matcherGroups[selectedMatcherIndex]; + } + return null; + }, [selectedHook, selectedMatcherIndex]); + useKeypress( (key) => { if (isLoading || loadError) { - // Allow Escape to close even during loading/error states if (key.name === 'escape') { onClose(); } @@ -210,8 +193,10 @@ export function HooksManagementDialog({ } else if (key.name === 'return') { if (hooks.length > 0 && listSelectedIndex >= 0) { setSelectedHookIndex(listSelectedIndex); + setSelectedMatcherIndex(-1); setSelectedConfigIndex(-1); setDetailSelectedIndex(0); + setMatcherSelectedIndex(0); setNavigationStack((prev) => [ ...prev, HOOKS_MANAGEMENT_STEPS.HOOK_DETAIL, @@ -225,15 +210,62 @@ export function HooksManagementDialog({ case HOOKS_MANAGEMENT_STEPS.HOOK_DETAIL: if (key.name === 'escape') { handleNavigateBack(); - } else if (selectedHook && selectedHook.configs.length > 0) { + } else if (selectedHook) { + const matcherMode = supportsMatchers(selectedHook.event); + if (matcherMode) { + if (selectedHook.matcherGroups.length === 0) { + break; + } + if (keyMatchers[Command.SELECTION_UP](key)) { + setDetailSelectedIndex((prev) => Math.max(0, prev - 1)); + } else if (keyMatchers[Command.SELECTION_DOWN](key)) { + setDetailSelectedIndex((prev) => + Math.min(selectedHook.matcherGroups.length - 1, prev + 1), + ); + } else if (key.name === 'return') { + setSelectedMatcherIndex(detailSelectedIndex); + setMatcherSelectedIndex(0); + setSelectedConfigIndex(-1); + setNavigationStack((prev) => [ + ...prev, + HOOKS_MANAGEMENT_STEPS.HOOK_MATCHER_DETAIL, + ]); + } + } else { + const flatConfigs = getAllConfigs(selectedHook); + if (flatConfigs.length === 0) { + break; + } + if (keyMatchers[Command.SELECTION_UP](key)) { + setDetailSelectedIndex((prev) => Math.max(0, prev - 1)); + } else if (keyMatchers[Command.SELECTION_DOWN](key)) { + setDetailSelectedIndex((prev) => + Math.min(flatConfigs.length - 1, prev + 1), + ); + } else if (key.name === 'return') { + setSelectedMatcherIndex(-1); + setSelectedConfigIndex(detailSelectedIndex); + setNavigationStack((prev) => [ + ...prev, + HOOKS_MANAGEMENT_STEPS.HOOK_CONFIG_DETAIL, + ]); + } + } + } + break; + + case HOOKS_MANAGEMENT_STEPS.HOOK_MATCHER_DETAIL: + if (key.name === 'escape') { + handleNavigateBack(); + } else if (selectedMatcher && selectedMatcher.configs.length > 0) { if (keyMatchers[Command.SELECTION_UP](key)) { - setDetailSelectedIndex((prev) => Math.max(0, prev - 1)); + setMatcherSelectedIndex((prev) => Math.max(0, prev - 1)); } else if (keyMatchers[Command.SELECTION_DOWN](key)) { - setDetailSelectedIndex((prev) => - Math.min(selectedHook.configs.length - 1, prev + 1), + setMatcherSelectedIndex((prev) => + Math.min(selectedMatcher.configs.length - 1, prev + 1), ); } else if (key.name === 'return') { - setSelectedConfigIndex(detailSelectedIndex); + setSelectedConfigIndex(matcherSelectedIndex); setNavigationStack((prev) => [ ...prev, HOOKS_MANAGEMENT_STEPS.HOOK_CONFIG_DETAIL, @@ -249,14 +281,12 @@ export function HooksManagementDialog({ break; default: - // No action for unknown steps break; } }, { isActive: true }, ); - // Load hooks data const fetchHooksData = useCallback((): HookEventDisplayInfo[] => { if (!config) return []; @@ -266,32 +296,36 @@ export function HooksManagementDialog({ SettingScope.Workspace, ).settings; - // Get translated source display map const sourceDisplayMap = getTranslatedSourceDisplayMap(); const result: HookEventDisplayInfo[] = []; for (const eventName of DISPLAY_HOOK_EVENTS) { const hookInfo = createEmptyHookEventInfo(eventName); + const groupByMatcher = supportsMatchers(eventName); - // Get hooks from user settings (with per-event validation) const userSettingsRecord = userSettings as Record; const userHooksRaw = userSettingsRecord?.['hooks']; if (isValidHooksRecord(userHooksRaw)) { const userDefs = getValidHookDefinitions(userHooksRaw, eventName); for (const def of userDefs) { for (const hookConfig of def.hooks) { - hookInfo.configs.push({ - config: hookConfig, - source: HooksConfigSource.User, - sourceDisplay: sourceDisplayMap[HooksConfigSource.User], - enabled: true, - }); + addConfigToMatcherGroup( + hookInfo, + def.matcher, + def.sequential, + { + config: hookConfig, + source: HooksConfigSource.User, + sourceDisplay: sourceDisplayMap[HooksConfigSource.User], + enabled: true, + }, + groupByMatcher, + ); } } } - // Get hooks from workspace settings (with per-event validation) const workspaceSettingsRecord = workspaceSettings as Record< string, unknown @@ -304,17 +338,22 @@ export function HooksManagementDialog({ ); for (const def of workspaceDefs) { for (const hookConfig of def.hooks) { - hookInfo.configs.push({ - config: hookConfig, - source: HooksConfigSource.Project, - sourceDisplay: sourceDisplayMap[HooksConfigSource.Project], - enabled: true, - }); + addConfigToMatcherGroup( + hookInfo, + def.matcher, + def.sequential, + { + config: hookConfig, + source: HooksConfigSource.Project, + sourceDisplay: sourceDisplayMap[HooksConfigSource.Project], + enabled: true, + }, + groupByMatcher, + ); } } } - // Get hooks from extensions (with type validation) const extensions = config.getExtensions() || []; for (const extension of extensions) { if (extension.isActive && extension.hooks?.[eventName]) { @@ -323,13 +362,19 @@ export function HooksManagementDialog({ for (const def of extensionHooks) { if (isValidHookDefinition(def)) { for (const hookConfig of def.hooks) { - hookInfo.configs.push({ - config: hookConfig, - source: HooksConfigSource.Extensions, - sourceDisplay: extension.name, - sourcePath: extension.path, - enabled: true, - }); + addConfigToMatcherGroup( + hookInfo, + def.matcher, + def.sequential, + { + config: hookConfig, + source: HooksConfigSource.Extensions, + sourceDisplay: extension.name, + sourcePath: extension.path, + enabled: true, + }, + groupByMatcher, + ); } } } @@ -337,7 +382,6 @@ export function HooksManagementDialog({ } } - // Get session hooks from SessionHooksManager const hookSystem = config.getHookSystem(); if (hookSystem) { const sessionId = config.getSessionId(); @@ -346,20 +390,23 @@ export function HooksManagementDialog({ const allSessionHooks = sessionHooksManager.getAllSessionHooks(sessionId); - // Filter hooks for this event const eventSessionHooks = allSessionHooks.filter( (hook: SessionHookEntry) => hook.eventName === eventName, ); for (const sessionHook of eventSessionHooks) { - // Session hooks have matcher stored separately from config - hookInfo.configs.push({ - config: sessionHook.config as HookConfig, - source: HooksConfigSource.Session, - sourceDisplay: t('Session (temporary)'), - matcher: sessionHook.matcher, - enabled: true, - }); + addConfigToMatcherGroup( + hookInfo, + sessionHook.matcher, + sessionHook.sequential, + { + config: sessionHook.config as HookConfig, + source: HooksConfigSource.Session, + sourceDisplay: t('Session (temporary)'), + enabled: true, + }, + groupByMatcher, + ); } } } @@ -370,7 +417,6 @@ export function HooksManagementDialog({ return result; }, [config]); - // Load hooks data on initial render useEffect(() => { let cancelled = false; setIsLoading(true); @@ -399,7 +445,6 @@ export function HooksManagementDialog({ }; }, [fetchHooksData]); - // Navigation handler for going back const handleNavigateBack = useCallback(() => { setNavigationStack((prev) => { if (prev.length <= 1) { @@ -410,27 +455,39 @@ export function HooksManagementDialog({ }); }, [onClose]); - // Selected hook config const selectedConfig = useMemo(() => { + if (!selectedHook) return null; + if (!supportsMatchers(selectedHook.event)) { + const flatConfigs = getAllConfigs(selectedHook); + if ( + selectedConfigIndex >= 0 && + selectedConfigIndex < flatConfigs.length + ) { + return flatConfigs[selectedConfigIndex]; + } + return null; + } if ( - selectedHook && + selectedMatcher && selectedConfigIndex >= 0 && - selectedConfigIndex < selectedHook.configs.length + selectedConfigIndex < selectedMatcher.configs.length ) { - return selectedHook.configs[selectedConfigIndex]; + return selectedMatcher.configs[selectedConfigIndex]; } return null; - }, [selectedHook, selectedConfigIndex]); + }, [selectedHook, selectedMatcher, selectedConfigIndex]); - // Calculate total configured hooks count const configuredHooksCount = useMemo( - () => hooks.reduce((sum, hook) => sum + hook.configs.length, 0), + () => + hooks.reduce( + (sum, hook) => + sum + hook.matcherGroups.reduce((s, g) => s + g.configs.length, 0), + 0, + ), [hooks], ); - // Render based on current step const renderContent = () => { - // Show disabled state first (before loading check) if (currentStep === HOOKS_MANAGEMENT_STEPS.HOOKS_DISABLED) { return ; } @@ -478,6 +535,22 @@ export function HooksManagementDialog({ ); + case HOOKS_MANAGEMENT_STEPS.HOOK_MATCHER_DETAIL: + if (selectedHook && selectedMatcher) { + return ( + + ); + } + return ( + + {t('No matcher selected')} + + ); + case HOOKS_MANAGEMENT_STEPS.HOOK_CONFIG_DETAIL: if (selectedHook && selectedConfig) { return ( diff --git a/packages/cli/src/ui/components/hooks/constants.test.ts b/packages/cli/src/ui/components/hooks/constants.test.ts index b24fec3b658..e4089c5f3c1 100644 --- a/packages/cli/src/ui/components/hooks/constants.test.ts +++ b/packages/cli/src/ui/components/hooks/constants.test.ts @@ -5,14 +5,16 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { HookEventName, HooksConfigSource } from '@qwen-code/qwen-code-core'; +import { + HookEventName, + HooksConfigSource, + hookEventSupportsMatcher, +} from '@qwen-code/qwen-code-core'; -// Mock i18n module vi.mock('../../../i18n/index.js', () => ({ t: vi.fn((key: string) => key), })); -// Import after mocking import { getHookExitCodes, getHookShortDescription, @@ -20,6 +22,7 @@ import { getTranslatedSourceDisplayMap, createEmptyHookEventInfo, DISPLAY_HOOK_EVENTS, + supportsMatchers, } from './constants.js'; describe('hooks constants', () => { @@ -83,6 +86,24 @@ describe('hooks constants', () => { expect(exitCodes).toHaveLength(3); }); + it('should return exit codes for PostCompact event', () => { + const exitCodes = getHookExitCodes(HookEventName.PostCompact); + expect(exitCodes).toHaveLength(2); + expect(exitCodes[0].code).toBe(0); + expect(exitCodes[1].code).toBe('Other'); + }); + + it('should return exit codes for StopFailure event', () => { + // Fire-and-forget per hookAggregator — both rows are documented as ignored. + const exitCodes = getHookExitCodes(HookEventName.StopFailure); + expect(exitCodes).toHaveLength(2); + expect(exitCodes[0].code).toBe(0); + expect(exitCodes[1].code).toBe('Other'); + for (const row of exitCodes) { + expect(row.description).toContain('fire-and-forget'); + } + }); + it('should return empty array for unknown event', () => { const exitCodes = getHookExitCodes('unknown_event' as HookEventName); expect(exitCodes).toEqual([]); @@ -110,6 +131,17 @@ describe('hooks constants', () => { expect(desc).toBe('When a new session is started'); }); + it('should return description for PostCompact', () => { + const desc = getHookShortDescription(HookEventName.PostCompact); + expect(desc).toBe('After conversation compaction'); + }); + + it('should return description for StopFailure', () => { + const desc = getHookShortDescription(HookEventName.StopFailure); + expect(desc).toContain('API error'); + expect(desc).toContain('Stop'); + }); + it('should return empty string for unknown event', () => { const desc = getHookShortDescription('unknown_event' as HookEventName); expect(desc).toBe(''); @@ -133,6 +165,19 @@ describe('hooks constants', () => { expect(desc).toBe(''); }); + it('should return description for PostCompact', () => { + const desc = getHookDescription(HookEventName.PostCompact); + expect(desc).toContain('trigger'); + expect(desc).toContain('compact_summary'); + }); + + it('should return description for StopFailure', () => { + const desc = getHookDescription(HookEventName.StopFailure); + expect(desc).toContain('error'); + expect(desc).toContain('rate_limit'); + expect(desc).toContain('Fire-and-forget'); + }); + it('should return empty string for unknown event', () => { const desc = getHookDescription('unknown_event' as HookEventName); expect(desc).toBe(''); @@ -152,7 +197,6 @@ describe('hooks constants', () => { it('should return translated strings', () => { const map = getTranslatedSourceDisplayMap(); - // All values should be strings (translated) Object.values(map).forEach((value) => { expect(typeof value).toBe('string'); expect(value.length).toBeGreaterThan(0); @@ -185,6 +229,40 @@ describe('hooks constants', () => { }); }); + describe('supportsMatchers', () => { + it('returns true for events with meaningful matchers', () => { + expect(supportsMatchers(HookEventName.PreToolUse)).toBe(true); + expect(supportsMatchers(HookEventName.PostToolUse)).toBe(true); + expect(supportsMatchers(HookEventName.PostToolUseFailure)).toBe(true); + expect(supportsMatchers(HookEventName.PermissionRequest)).toBe(true); + expect(supportsMatchers(HookEventName.Notification)).toBe(true); + expect(supportsMatchers(HookEventName.SessionStart)).toBe(true); + expect(supportsMatchers(HookEventName.SessionEnd)).toBe(true); + expect(supportsMatchers(HookEventName.SubagentStart)).toBe(true); + expect(supportsMatchers(HookEventName.SubagentStop)).toBe(true); + expect(supportsMatchers(HookEventName.PreCompact)).toBe(true); + expect(supportsMatchers(HookEventName.PostCompact)).toBe(true); + expect(supportsMatchers(HookEventName.StopFailure)).toBe(true); + }); + + it('returns false for events without matchers', () => { + expect(supportsMatchers(HookEventName.Stop)).toBe(false); + expect(supportsMatchers(HookEventName.UserPromptSubmit)).toBe(false); + expect(supportsMatchers(HookEventName.TodoCreated)).toBe(false); + expect(supportsMatchers(HookEventName.TodoCompleted)).toBe(false); + }); + + it('returns false for unknown events', () => { + expect(supportsMatchers('unknown_event' as HookEventName)).toBe(false); + }); + + it('covers every HookEventName value and matches core dispatch', () => { + for (const event of Object.values(HookEventName)) { + expect(supportsMatchers(event)).toBe(hookEventSupportsMatcher(event)); + } + }); + }); + describe('createEmptyHookEventInfo', () => { it('should create empty info for PreToolUse', () => { const info = createEmptyHookEventInfo(HookEventName.PreToolUse); @@ -195,7 +273,7 @@ describe('hooks constants', () => { 'Input to command is JSON of tool call arguments.', ); expect(info.exitCodes).toHaveLength(3); - expect(info.configs).toEqual([]); + expect(info.matcherGroups).toEqual([]); }); it('should create empty info for Stop', () => { @@ -207,7 +285,7 @@ describe('hooks constants', () => { ); expect(info.description).toBe(''); expect(info.exitCodes).toHaveLength(3); - expect(info.configs).toEqual([]); + expect(info.matcherGroups).toEqual([]); }); it('should create empty info for unknown event', () => { @@ -217,7 +295,7 @@ describe('hooks constants', () => { expect(info.shortDescription).toBe(''); expect(info.description).toBe(''); expect(info.exitCodes).toEqual([]); - expect(info.configs).toEqual([]); + expect(info.matcherGroups).toEqual([]); }); it('should create empty info for TodoCreated', () => { @@ -227,7 +305,7 @@ describe('hooks constants', () => { expect(info.shortDescription).toBe('When a new todo item is created'); expect(info.description).toContain('todo_id'); expect(info.exitCodes).toHaveLength(3); - expect(info.configs).toEqual([]); + expect(info.matcherGroups).toEqual([]); }); it('should create empty info for TodoCompleted', () => { @@ -239,7 +317,7 @@ describe('hooks constants', () => { ); expect(info.description).toContain('previous_status'); expect(info.exitCodes).toHaveLength(3); - expect(info.configs).toEqual([]); + expect(info.matcherGroups).toEqual([]); }); }); }); diff --git a/packages/cli/src/ui/components/hooks/constants.ts b/packages/cli/src/ui/components/hooks/constants.ts index a80ca9cc2c8..ef8b9013ea7 100644 --- a/packages/cli/src/ui/components/hooks/constants.ts +++ b/packages/cli/src/ui/components/hooks/constants.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { HooksConfigSource, HookEventName } from '@qwen-code/qwen-code-core'; +import { + HooksConfigSource, + HookEventName, + hookEventSupportsMatcher, +} from '@qwen-code/qwen-code-core'; import type { HookExitCode, HookEventDisplayInfo } from './types.js'; import { t } from '../../../i18n/index.js'; @@ -90,6 +94,20 @@ export function getHookExitCodes(eventName: string): HookExitCode[] { description: t('show stderr to user only but continue with compaction'), }, ], + [HookEventName.PostCompact]: [ + { code: 0, description: t('stdout/stderr not shown') }, + { code: 'Other', description: t('show stderr to user only') }, + ], + [HookEventName.StopFailure]: [ + { + code: 0, + description: t('fire-and-forget; exit status is ignored'), + }, + { + code: 'Other', + description: t('fire-and-forget; exit status is ignored'), + }, + ], [HookEventName.PermissionRequest]: [ { code: 0, description: t('use hook decision if provided') }, { code: 'Other', description: t('show stderr to user only') }, @@ -133,6 +151,10 @@ export function getHookShortDescription(eventName: string): string { 'Right before a subagent concludes its response', ), [HookEventName.PreCompact]: t('Before conversation compaction'), + [HookEventName.PostCompact]: t('After conversation compaction'), + [HookEventName.StopFailure]: t( + 'When the turn ends due to an API error (fires instead of Stop)', + ), [HookEventName.SessionEnd]: t('When a session is ending'), [HookEventName.PermissionRequest]: t( 'When a permission dialog is displayed', @@ -179,6 +201,12 @@ export function getHookDescription(eventName: string): string { [HookEventName.PreCompact]: t( 'Input to command is JSON with compaction details.', ), + [HookEventName.PostCompact]: t( + 'Input to command is JSON with trigger (manual/auto) and compact_summary. Output is ignored for control purposes.', + ), + [HookEventName.StopFailure]: t( + 'Input to command is JSON with error (rate_limit, authentication_failed, billing_error, invalid_request, server_error, max_output_tokens, unknown) and optional error_details. Fire-and-forget: output and exit status are ignored.', + ), [HookEventName.PermissionRequest]: t( 'Input to command is JSON with tool_name, tool_input, and tool_use_id. Output JSON with hookSpecificOutput containing decision to allow or deny.', ), @@ -208,19 +236,13 @@ export function getTranslatedSourceDisplayMap(): Record< }; } -/** - * List of hook events to display in the UI - * Automatically synced with HookEventName enum from core. - * Note: Order follows the enum definition order. If UI presentation order - * needs to be different (e.g., grouped by lifecycle phase), consider using - * an explicit sorted array instead. Current enum order is acceptable for display. - */ export const DISPLAY_HOOK_EVENTS: HookEventName[] = Object.values(HookEventName); -/** - * Create empty hook event display info - */ +export function supportsMatchers(eventName: HookEventName): boolean { + return hookEventSupportsMatcher(eventName); +} + export function createEmptyHookEventInfo( eventName: HookEventName, ): HookEventDisplayInfo { @@ -229,6 +251,6 @@ export function createEmptyHookEventInfo( shortDescription: getHookShortDescription(eventName), description: getHookDescription(eventName), exitCodes: getHookExitCodes(eventName), - configs: [], + matcherGroups: [], }; } diff --git a/packages/cli/src/ui/components/hooks/matcherGrouping.test.ts b/packages/cli/src/ui/components/hooks/matcherGrouping.test.ts new file mode 100644 index 00000000000..5450f5df45b --- /dev/null +++ b/packages/cli/src/ui/components/hooks/matcherGrouping.test.ts @@ -0,0 +1,244 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + HookEventName, + HooksConfigSource, + HookType, +} from '@qwen-code/qwen-code-core'; +import type { HookEventDisplayInfo } from './types.js'; +import { + addConfigToMatcherGroup, + getAllConfigs, + normalizeMatcher, +} from './matcherGrouping.js'; + +function emptyHookInfo(): HookEventDisplayInfo { + return { + event: HookEventName.PreToolUse, + shortDescription: '', + description: '', + exitCodes: [], + matcherGroups: [], + }; +} + +describe('normalizeMatcher', () => { + it('returns "*" when matcher is undefined', () => { + expect(normalizeMatcher(undefined)).toBe('*'); + }); + + it('returns "*" when matcher is empty string', () => { + expect(normalizeMatcher('')).toBe('*'); + }); + + it('returns "*" when matcher is only whitespace', () => { + expect(normalizeMatcher(' ')).toBe('*'); + }); + + it('returns the trimmed matcher when set', () => { + expect(normalizeMatcher(' Bash ')).toBe('Bash'); + }); + + it('preserves regex-style matchers', () => { + expect(normalizeMatcher('Edit|Write')).toBe('Edit|Write'); + }); +}); + +describe('addConfigToMatcherGroup', () => { + it('creates a new group for an unseen matcher', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, 'Bash', undefined, { + config: { type: HookType.Command, command: '/x.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(info.matcherGroups).toHaveLength(1); + expect(info.matcherGroups[0].matcher).toBe('Bash'); + expect(info.matcherGroups[0].configs).toHaveLength(1); + expect(getAllConfigs(info)).toHaveLength(1); + }); + + it('reuses the existing group for the same matcher', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, 'Bash', undefined, { + config: { type: HookType.Command, command: '/a.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, 'Bash', undefined, { + config: { type: HookType.Command, command: '/b.sh' }, + source: HooksConfigSource.Project, + sourceDisplay: 'Local Settings', + enabled: true, + }); + + expect(info.matcherGroups).toHaveLength(1); + expect(info.matcherGroups[0].configs).toHaveLength(2); + expect(getAllConfigs(info)).toHaveLength(2); + }); + + it('buckets undefined / empty matchers into "*"', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, undefined, undefined, { + config: { type: HookType.Command, command: '/a.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, '', undefined, { + config: { type: HookType.Command, command: '/b.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(info.matcherGroups).toHaveLength(1); + expect(info.matcherGroups[0].matcher).toBe('*'); + expect(info.matcherGroups[0].configs).toHaveLength(2); + }); + + it('writes the normalized matcher onto the stored config', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, undefined, undefined, { + config: { type: HookType.Command, command: '/x.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(getAllConfigs(info)[0].matcher).toBe('*'); + expect(info.matcherGroups[0].configs[0].matcher).toBe('*'); + }); + + it('promotes group.sequential to true when any handler is sequential', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, 'Bash', false, { + config: { type: HookType.Command, command: '/a.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, 'Bash', true, { + config: { type: HookType.Command, command: '/b.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(info.matcherGroups[0].sequential).toBe(true); + }); + + it('normalizes missing sequential to a false boolean, not undefined', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, 'Bash', undefined, { + config: { type: HookType.Command, command: '/a.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, 'Bash', false, { + config: { type: HookType.Command, command: '/b.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(info.matcherGroups[0].sequential).toBe(false); + expect(info.matcherGroups[0].sequential).not.toBe(undefined); + const flat = getAllConfigs(info); + expect(flat[0].sequential).toBe(false); + expect(flat[1].sequential).toBe(false); + }); + + it('preserves insertion order across matchers', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup(info, 'Bash', undefined, { + config: { type: HookType.Command, command: '/a.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, 'Edit|Write', undefined, { + config: { type: HookType.Command, command: '/b.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + addConfigToMatcherGroup(info, undefined, undefined, { + config: { type: HookType.Command, command: '/c.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }); + + expect(info.matcherGroups.map((g) => g.matcher)).toEqual([ + 'Bash', + 'Edit|Write', + '*', + ]); + }); + + it('keeps non-matcher events in original handler order', () => { + const info = emptyHookInfo(); + + addConfigToMatcherGroup( + info, + 'A', + undefined, + { + config: { type: HookType.Command, command: '/first.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }, + false, + ); + addConfigToMatcherGroup( + info, + 'B', + undefined, + { + config: { type: HookType.Command, command: '/second.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }, + false, + ); + addConfigToMatcherGroup( + info, + 'A', + undefined, + { + config: { type: HookType.Command, command: '/third.sh' }, + source: HooksConfigSource.User, + sourceDisplay: 'User Settings', + enabled: true, + }, + false, + ); + + expect(info.matcherGroups).toHaveLength(1); + expect(info.matcherGroups[0].matcher).toBe('*'); + expect( + getAllConfigs(info).map((config) => + config.config.type === HookType.Command ? config.config.command : '', + ), + ).toEqual(['/first.sh', '/second.sh', '/third.sh']); + }); +}); diff --git a/packages/cli/src/ui/components/hooks/matcherGrouping.ts b/packages/cli/src/ui/components/hooks/matcherGrouping.ts new file mode 100644 index 00000000000..0e8d24d6795 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/matcherGrouping.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { HookConfigDisplayInfo, HookEventDisplayInfo } from './types.js'; + +export function normalizeMatcher(matcher?: string): string { + const trimmed = matcher?.trim(); + return trimmed ? trimmed : '*'; +} + +export function addConfigToMatcherGroup( + hookInfo: HookEventDisplayInfo, + matcher: string | undefined, + sequential: boolean | undefined, + configInfo: HookConfigDisplayInfo, + groupByMatcher = true, +): void { + const normalizedMatcher = groupByMatcher ? normalizeMatcher(matcher) : '*'; + const normalizedSequential = sequential ?? false; + const normalizedConfig: HookConfigDisplayInfo = { + ...configInfo, + matcher: normalizedMatcher, + sequential: normalizedSequential, + }; + + let group = hookInfo.matcherGroups.find( + (candidate) => candidate.matcher === normalizedMatcher, + ); + if (!group) { + group = { + matcher: normalizedMatcher, + sequential: normalizedSequential, + configs: [], + }; + hookInfo.matcherGroups.push(group); + } else if (normalizedSequential) { + group.sequential = true; + } + + group.configs.push(normalizedConfig); +} + +export function getAllConfigs( + hookInfo: HookEventDisplayInfo, +): HookConfigDisplayInfo[] { + return hookInfo.matcherGroups.flatMap((group) => group.configs); +} diff --git a/packages/cli/src/ui/components/hooks/sourceLabels.test.ts b/packages/cli/src/ui/components/hooks/sourceLabels.test.ts new file mode 100644 index 00000000000..4c5c39c14b7 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/sourceLabels.test.ts @@ -0,0 +1,115 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HooksConfigSource, HookType } from '@qwen-code/qwen-code-core'; + +vi.mock('../../../i18n/index.js', () => ({ + t: vi.fn((key: string) => key), +})); + +import { + formatSourceLabel, + formatSourceLabels, + getConfigSourceDisplay, +} from './sourceLabels.js'; +import type { HookConfigDisplayInfo } from './types.js'; + +function makeConfig( + source: HooksConfigSource, + sourceDisplay = '', +): HookConfigDisplayInfo { + return { + config: { type: HookType.Command, command: '/x.sh' }, + source, + sourceDisplay, + enabled: true, + }; +} + +describe('sourceLabels', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('formatSourceLabel', () => { + it('returns short label for each known source', () => { + expect(formatSourceLabel(HooksConfigSource.User)).toBe('User'); + expect(formatSourceLabel(HooksConfigSource.Project)).toBe('Project'); + expect(formatSourceLabel(HooksConfigSource.System)).toBe('System'); + expect(formatSourceLabel(HooksConfigSource.Extensions)).toBe('Extension'); + expect(formatSourceLabel(HooksConfigSource.Session)).toBe('Session'); + }); + + it('falls back to the raw source string for unknown values', () => { + expect(formatSourceLabel('mystery' as HooksConfigSource)).toBe('mystery'); + }); + }); + + describe('formatSourceLabels', () => { + it('returns a single label for a single-source group', () => { + const configs = [ + makeConfig(HooksConfigSource.User), + makeConfig(HooksConfigSource.User), + ]; + expect(formatSourceLabels(configs)).toBe('User'); + }); + + it('joins distinct labels with comma + space for multi-source groups', () => { + const configs = [ + makeConfig(HooksConfigSource.User), + makeConfig(HooksConfigSource.Project), + makeConfig(HooksConfigSource.Extensions), + ]; + expect(formatSourceLabels(configs)).toBe('User, Project, Extension'); + }); + + it('preserves insertion order across distinct sources', () => { + const configs = [ + makeConfig(HooksConfigSource.Project), + makeConfig(HooksConfigSource.User), + ]; + expect(formatSourceLabels(configs)).toBe('Project, User'); + }); + + it('returns empty string when there are no configs', () => { + expect(formatSourceLabels([])).toBe(''); + }); + }); + + describe('getConfigSourceDisplay', () => { + it('returns the translated long label for non-extension sources', () => { + expect(getConfigSourceDisplay(makeConfig(HooksConfigSource.User))).toBe( + 'User Settings', + ); + expect( + getConfigSourceDisplay(makeConfig(HooksConfigSource.Project)), + ).toBe('Local Settings'); + expect(getConfigSourceDisplay(makeConfig(HooksConfigSource.System))).toBe( + 'System Settings', + ); + }); + + it('appends the extension name for Extensions-source configs', () => { + expect( + getConfigSourceDisplay( + makeConfig(HooksConfigSource.Extensions, 'my-ext'), + ), + ).toBe('Extensions (my-ext)'); + }); + + it('uses the long "Session (temporary)" label for session-source configs', () => { + expect( + getConfigSourceDisplay(makeConfig(HooksConfigSource.Session)), + ).toBe('Session (temporary)'); + }); + + it('falls back to the raw source string for unknown sources', () => { + const config = makeConfig('mystery' as HooksConfigSource); + expect(getConfigSourceDisplay(config)).toBe('mystery'); + }); + }); +}); diff --git a/packages/cli/src/ui/components/hooks/sourceLabels.ts b/packages/cli/src/ui/components/hooks/sourceLabels.ts new file mode 100644 index 00000000000..64c3dd219f5 --- /dev/null +++ b/packages/cli/src/ui/components/hooks/sourceLabels.ts @@ -0,0 +1,44 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { HooksConfigSource } from '@qwen-code/qwen-code-core'; +import type { HookConfigDisplayInfo } from './types.js'; +import { getTranslatedSourceDisplayMap } from './constants.js'; +import { t } from '../../../i18n/index.js'; + +export function formatSourceLabel(source: HooksConfigSource): string { + switch (source) { + case HooksConfigSource.User: + return t('User'); + case HooksConfigSource.Project: + return t('Project'); + case HooksConfigSource.System: + return t('System'); + case HooksConfigSource.Extensions: + return t('Extension'); + case HooksConfigSource.Session: + return t('Session'); + default: + return source; + } +} + +export function formatSourceLabels(configs: HookConfigDisplayInfo[]): string { + return Array.from( + new Set(configs.map((config) => formatSourceLabel(config.source))), + ).join(', '); +} + +export function getConfigSourceDisplay(config: { + source: HooksConfigSource; + sourceDisplay: string; +}): string { + const sourceDisplayMap = getTranslatedSourceDisplayMap(); + if (config.source === HooksConfigSource.Extensions) { + return `${sourceDisplayMap[HooksConfigSource.Extensions]} (${config.sourceDisplay})`; + } + return sourceDisplayMap[config.source] || config.source; +} diff --git a/packages/cli/src/ui/components/hooks/types.ts b/packages/cli/src/ui/components/hooks/types.ts index a00ac0f2456..f5ec990432e 100644 --- a/packages/cli/src/ui/components/hooks/types.ts +++ b/packages/cli/src/ui/components/hooks/types.ts @@ -10,53 +10,46 @@ import type { HookEventName, } from '@qwen-code/qwen-code-core'; -/** - * Exit code description for hooks - */ export interface HookExitCode { code: number | string; description: string; } -/** - * UI display information for a hook event - */ export interface HookEventDisplayInfo { event: HookEventName; shortDescription: string; description: string; exitCodes: HookExitCode[]; + matcherGroups: HookMatcherDisplayInfo[]; +} + +export interface HookMatcherDisplayInfo { + matcher: string; + sequential?: boolean; configs: HookConfigDisplayInfo[]; } -/** - * UI display information for a hook configuration - */ export interface HookConfigDisplayInfo { config: HookConfig; source: HooksConfigSource; sourceDisplay: string; sourcePath?: string; matcher?: string; + sequential?: boolean; enabled: boolean; } -/** - * Hook management dialog step names - */ export const HOOKS_MANAGEMENT_STEPS = { HOOKS_DISABLED: 'hooks_disabled', HOOKS_LIST: 'hooks_list', HOOK_DETAIL: 'hook_detail', + HOOK_MATCHER_DETAIL: 'hook_matcher_detail', HOOK_CONFIG_DETAIL: 'hook_config_detail', } as const; export type HooksManagementStep = (typeof HOOKS_MANAGEMENT_STEPS)[keyof typeof HOOKS_MANAGEMENT_STEPS]; -/** - * Props for HooksManagementDialog - */ export interface HooksManagementDialogProps { onClose: () => void; } diff --git a/packages/core/src/hooks/hookPlanner.ts b/packages/core/src/hooks/hookPlanner.ts index e6395f2657b..def94b19ef2 100644 --- a/packages/core/src/hooks/hookPlanner.ts +++ b/packages/core/src/hooks/hookPlanner.ts @@ -69,6 +69,11 @@ export function getHookMatcherTarget( } } +export function hookEventSupportsMatcher(eventName: HookEventName): boolean { + const target = getHookMatcherTarget(eventName); + return typeof target === 'object' && target !== null; +} + /** * Hook planner that selects matching hooks and creates execution plans */ diff --git a/packages/core/src/hooks/index.ts b/packages/core/src/hooks/index.ts index 5f7607dbb1e..0580ebb1af5 100644 --- a/packages/core/src/hooks/index.ts +++ b/packages/core/src/hooks/index.ts @@ -12,7 +12,7 @@ export { HookSystem } from './hookSystem.js'; export { HookRegistry } from './hookRegistry.js'; export { HookRunner } from './hookRunner.js'; export { HookAggregator } from './hookAggregator.js'; -export { HookPlanner } from './hookPlanner.js'; +export { HookPlanner, hookEventSupportsMatcher } from './hookPlanner.js'; export { HookEventHandler } from './hookEventHandler.js'; // Export new hook runners diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 336f23ff338..08dffbc78ec 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -388,7 +388,11 @@ export * from './test-utils/index.js'; // ============================================================================ export * from './hooks/types.js'; -export { HookSystem, HookRegistry } from './hooks/index.js'; +export { + HookSystem, + HookRegistry, + hookEventSupportsMatcher, +} from './hooks/index.js'; export type { HookRegistryEntry, SessionHookEntry } from './hooks/index.js'; export { DEFAULT_STOP_HOOK_BLOCK_CAP, From e32a113b8ea59304ff7de606feef5ceabcff2bfa Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 26 May 2026 22:38:53 +0800 Subject: [PATCH 2/2] test(cli): cover hooks navigation levels --- packages/cli/src/ui/commands/hooksCommand.ts | 6 +- .../hooks/HooksManagementDialog.test.tsx | 162 +++++++++++++++++- 2 files changed, 162 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/ui/commands/hooksCommand.ts b/packages/cli/src/ui/commands/hooksCommand.ts index 748c477bae4..7d1f10d0192 100644 --- a/packages/cli/src/ui/commands/hooksCommand.ts +++ b/packages/cli/src/ui/commands/hooksCommand.ts @@ -18,6 +18,7 @@ import type { 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 @@ -91,11 +92,6 @@ const listCommand: SlashCommand = { }; } - const normalizeMatcher = (matcher: string | undefined): string => { - const trimmed = matcher?.trim(); - return trimmed ? trimmed : '*'; - }; - interface FlattenedHook { name: string; source: string; diff --git a/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx b/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx index 321596b4b5b..e4fd15235f3 100644 --- a/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx +++ b/packages/cli/src/ui/components/hooks/HooksManagementDialog.test.tsx @@ -5,10 +5,12 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { cleanup } from 'ink-testing-library'; import { HooksManagementDialog } from './HooksManagementDialog.js'; import { renderWithProviders } from '../../../test-utils/render.js'; import { useKeypress } from '../../hooks/useKeypress.js'; import { useConfig } from '../../contexts/ConfigContext.js'; +import { loadSettings, SettingScope } from '../../../config/settings.js'; import type { Key } from '../../contexts/KeypressContext.js'; vi.mock('../../hooks/useKeypress.js', () => ({ @@ -17,6 +19,8 @@ vi.mock('../../hooks/useKeypress.js', () => ({ const mockedUseKeypress = vi.mocked(useKeypress); const mockedUseConfig = vi.mocked(useConfig); +const mockedLoadSettings = vi.mocked(loadSettings); +let keypressHandler: ((key: Key) => void) | null = null; /** * Returns a `useConfig` return value with `disableAllHooks` flipped on, while @@ -140,9 +144,23 @@ function createKey(name: string, sequence = ''): Key { }; } +function mockSettingsHooks(userHooks: Record): void { + mockedLoadSettings.mockReturnValue({ + forScope: vi.fn((scope: SettingScope) => ({ + settings: + scope === SettingScope.User ? { hooks: userHooks } : { hooks: {} }, + })), + } as unknown as ReturnType); +} + +function pressKey(name: string, sequence = ''): void { + const latestHandler = mockedUseKeypress.mock.calls.at(-1)?.[0]; + expect(latestHandler).toBeDefined(); + latestHandler!(createKey(name, sequence)); +} + describe('HooksManagementDialog', () => { const mockOnClose = vi.fn(); - let keypressHandler: ((key: Key) => void) | null = null; beforeEach(() => { vi.clearAllMocks(); @@ -155,6 +173,7 @@ describe('HooksManagementDialog', () => { afterEach(() => { keypressHandler = null; + cleanup(); }); it('should render loading state initially', () => { @@ -204,4 +223,145 @@ describe('HooksManagementDialog', () => { expect(mockOnClose).toHaveBeenCalledTimes(1); }); + + it('should navigate from a matcher hook to matcher detail', async () => { + mockSettingsHooks({ + PreToolUse: [ + { + matcher: 'Read', + hooks: [{ type: 'command', command: 'echo read' }], + }, + { + matcher: 'Bash', + hooks: [{ type: 'command', command: 'echo bash' }], + }, + ], + }); + + const { lastFrame } = renderWithProviders( + , + ); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('Hooks'); + }); + + pressKey('return'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('[User] Read'); + }); + + pressKey('down'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('❯ 2. [User] Bash'); + }); + pressKey('return'); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('PreToolUse - Matcher: Bash'); + expect(lastFrame()).toContain('echo bash'); + }); + + pressKey('escape', '\x1b'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('PreToolUse - Matchers'); + }); + }); + + it('should navigate from matcher detail to config detail', async () => { + mockSettingsHooks({ + PreToolUse: [ + { + matcher: 'Read', + hooks: [{ type: 'command', command: 'echo read' }], + }, + { + matcher: 'Bash', + hooks: [ + { type: 'command', command: 'echo first' }, + { type: 'command', command: 'echo second' }, + ], + }, + ], + }); + + const { lastFrame } = renderWithProviders( + , + ); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('Hooks'); + }); + + pressKey('return'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('[User] Read'); + }); + pressKey('down'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('❯ 2. [User] Bash'); + }); + pressKey('return'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('PreToolUse - Matcher: Bash'); + }); + + pressKey('down'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('❯ 2. [command] echo second'); + }); + pressKey('return'); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('Hook details'); + expect(lastFrame()).toContain('echo second'); + }); + }); + + it('should navigate directly from a non-matcher hook to config detail', async () => { + mockSettingsHooks({ + Stop: [ + { + hooks: [{ type: 'command', command: 'echo stop one' }], + }, + { + hooks: [{ type: 'command', command: 'echo stop two' }], + }, + ], + }); + + const { lastFrame } = renderWithProviders( + , + ); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('Hooks'); + }); + + for (let i = 0; i < 6; i++) { + pressKey('down'); + await vi.waitFor(() => { + expect(lastFrame()).toContain(`❯ ${i + 2}.`); + }); + } + await vi.waitFor(() => { + expect(lastFrame()).toContain('❯ 7. Stop'); + }); + pressKey('return'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('Stop'); + expect(lastFrame()).toContain('echo stop one'); + }); + + pressKey('down'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('❯ 2. [command] echo stop two'); + }); + pressKey('return'); + + await vi.waitFor(() => { + expect(lastFrame()).toContain('Hook details'); + expect(lastFrame()).toContain('echo stop two'); + }); + }); });