feat(hooks): Hooks Commands Panel, Enable/Disable, and Migrate (#14225)

This commit is contained in:
Edilmo Palencia
2025-12-03 10:01:57 -08:00
committed by GitHub
parent 08067acc71
commit b8c038f41f
24 changed files with 2568 additions and 16 deletions
@@ -0,0 +1,569 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import { hooksCommand } from './hooksCommand.js';
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
import { MessageType } from '../types.js';
import type { HookRegistryEntry } from '@google/gemini-cli-core';
import { HookType, HookEventName, ConfigSource } from '@google/gemini-cli-core';
import type { CommandContext } from './types.js';
describe('hooksCommand', () => {
let mockContext: CommandContext;
let mockHookSystem: {
getAllHooks: ReturnType<typeof vi.fn>;
setHookEnabled: ReturnType<typeof vi.fn>;
getRegistry: ReturnType<typeof vi.fn>;
};
let mockConfig: {
getHookSystem: ReturnType<typeof vi.fn>;
};
let mockSettings: {
merged: {
hooks?: {
disabled?: string[];
};
};
setValue: ReturnType<typeof vi.fn>;
};
beforeEach(() => {
vi.clearAllMocks();
// Create mock hook system
mockHookSystem = {
getAllHooks: vi.fn().mockReturnValue([]),
setHookEnabled: vi.fn(),
getRegistry: vi.fn().mockReturnValue({
initialize: vi.fn().mockResolvedValue(undefined),
}),
};
// Create mock config
mockConfig = {
getHookSystem: vi.fn().mockReturnValue(mockHookSystem),
};
// Create mock settings
mockSettings = {
merged: {
hooks: {
disabled: [],
},
},
setValue: vi.fn(),
};
// Create mock context with config and settings
mockContext = createMockCommandContext({
services: {
config: mockConfig,
settings: mockSettings,
},
});
});
afterEach(() => {
vi.restoreAllMocks();
});
describe('root command', () => {
it('should have the correct name and description', () => {
expect(hooksCommand.name).toBe('hooks');
expect(hooksCommand.description).toBe('Manage hooks');
});
it('should have all expected subcommands', () => {
expect(hooksCommand.subCommands).toBeDefined();
expect(hooksCommand.subCommands).toHaveLength(3);
const subCommandNames = hooksCommand.subCommands!.map((cmd) => cmd.name);
expect(subCommandNames).toContain('panel');
expect(subCommandNames).toContain('enable');
expect(subCommandNames).toContain('disable');
});
it('should delegate to panel action when invoked without subcommand', async () => {
if (!hooksCommand.action) {
throw new Error('hooks command must have an action');
}
mockHookSystem.getAllHooks.mockReturnValue([
createMockHook('test-hook', HookEventName.BeforeTool, true),
]);
await hooksCommand.action(mockContext, '');
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageType.HOOKS_LIST,
}),
expect.any(Number),
);
});
});
describe('panel subcommand', () => {
it('should return error when config is not loaded', async () => {
const contextWithoutConfig = createMockCommandContext({
services: {
config: null,
},
});
const panelCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'panel',
);
if (!panelCmd?.action) {
throw new Error('panel command must have an action');
}
const result = await panelCmd.action(contextWithoutConfig, '');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
});
});
it('should return info message when hook system is not enabled', async () => {
mockConfig.getHookSystem.mockReturnValue(null);
const panelCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'panel',
);
if (!panelCmd?.action) {
throw new Error('panel command must have an action');
}
const result = await panelCmd.action(mockContext, '');
expect(result).toEqual({
type: 'message',
messageType: 'info',
content:
'Hook system is not enabled. Enable it in settings with tools.enableHooks',
});
});
it('should return info message when no hooks are configured', async () => {
mockHookSystem.getAllHooks.mockReturnValue([]);
const panelCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'panel',
);
if (!panelCmd?.action) {
throw new Error('panel command must have an action');
}
const result = await panelCmd.action(mockContext, '');
expect(result).toEqual({
type: 'message',
messageType: 'info',
content:
'No hooks configured. Add hooks to your settings to get started.',
});
});
it('should display hooks list when hooks are configured', async () => {
const mockHooks: HookRegistryEntry[] = [
createMockHook('echo-test', HookEventName.BeforeTool, true),
createMockHook('notify', HookEventName.AfterAgent, false),
];
mockHookSystem.getAllHooks.mockReturnValue(mockHooks);
const panelCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'panel',
);
if (!panelCmd?.action) {
throw new Error('panel command must have an action');
}
await panelCmd.action(mockContext, '');
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageType.HOOKS_LIST,
hooks: mockHooks,
}),
expect.any(Number),
);
});
});
describe('enable subcommand', () => {
it('should return error when config is not loaded', async () => {
const contextWithoutConfig = createMockCommandContext({
services: {
config: null,
},
});
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.action) {
throw new Error('enable command must have an action');
}
const result = await enableCmd.action(contextWithoutConfig, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
});
});
it('should return error when hook system is not enabled', async () => {
mockConfig.getHookSystem.mockReturnValue(null);
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.action) {
throw new Error('enable command must have an action');
}
const result = await enableCmd.action(mockContext, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Hook system is not enabled.',
});
});
it('should return error when hook name is not provided', async () => {
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.action) {
throw new Error('enable command must have an action');
}
const result = await enableCmd.action(mockContext, '');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Usage: /hooks enable <hook-name>',
});
});
it('should enable a hook and update settings', async () => {
// Update the context's settings with disabled hooks
mockContext.services.settings.merged.hooks = {
disabled: ['test-hook', 'other-hook'],
};
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.action) {
throw new Error('enable command must have an action');
}
const result = await enableCmd.action(mockContext, 'test-hook');
expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
expect.any(String),
'hooks.disabled',
['other-hook'],
);
expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(
'test-hook',
true,
);
expect(result).toEqual({
type: 'message',
messageType: 'info',
content: 'Hook "test-hook" enabled successfully.',
});
});
it('should handle error when enabling hook fails', async () => {
mockSettings.setValue.mockImplementationOnce(() => {
throw new Error('Failed to save settings');
});
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.action) {
throw new Error('enable command must have an action');
}
const result = await enableCmd.action(mockContext, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Failed to enable hook: Failed to save settings',
});
});
});
describe('disable subcommand', () => {
it('should return error when config is not loaded', async () => {
const contextWithoutConfig = createMockCommandContext({
services: {
config: null,
},
});
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(contextWithoutConfig, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
});
});
it('should return error when hook system is not enabled', async () => {
mockConfig.getHookSystem.mockReturnValue(null);
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(mockContext, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Hook system is not enabled.',
});
});
it('should return error when hook name is not provided', async () => {
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(mockContext, '');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Usage: /hooks disable <hook-name>',
});
});
it('should disable a hook and update settings', async () => {
mockContext.services.settings.merged.hooks = {
disabled: [],
};
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(mockContext, 'test-hook');
expect(mockContext.services.settings.setValue).toHaveBeenCalledWith(
expect.any(String),
'hooks.disabled',
['test-hook'],
);
expect(mockHookSystem.setHookEnabled).toHaveBeenCalledWith(
'test-hook',
false,
);
expect(result).toEqual({
type: 'message',
messageType: 'info',
content: 'Hook "test-hook" disabled successfully.',
});
});
it('should return info when hook is already disabled', async () => {
// Update the context's settings with the hook already disabled
mockContext.services.settings.merged.hooks = {
disabled: ['test-hook'],
};
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(mockContext, 'test-hook');
expect(mockContext.services.settings.setValue).not.toHaveBeenCalled();
expect(mockHookSystem.setHookEnabled).not.toHaveBeenCalled();
expect(result).toEqual({
type: 'message',
messageType: 'info',
content: 'Hook "test-hook" is already disabled.',
});
});
it('should handle error when disabling hook fails', async () => {
mockContext.services.settings.merged.hooks = {
disabled: [],
};
mockSettings.setValue.mockImplementationOnce(() => {
throw new Error('Failed to save settings');
});
const disableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'disable',
);
if (!disableCmd?.action) {
throw new Error('disable command must have an action');
}
const result = await disableCmd.action(mockContext, 'test-hook');
expect(result).toEqual({
type: 'message',
messageType: 'error',
content: 'Failed to disable hook: Failed to save settings',
});
});
});
describe('completion', () => {
it('should return empty array when config is not available', () => {
const contextWithoutConfig = createMockCommandContext({
services: {
config: null,
},
});
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.completion) {
throw new Error('enable command must have completion');
}
const result = enableCmd.completion(contextWithoutConfig, 'test');
expect(result).toEqual([]);
});
it('should return empty array when hook system is not enabled', () => {
mockConfig.getHookSystem.mockReturnValue(null);
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.completion) {
throw new Error('enable command must have completion');
}
const result = enableCmd.completion(mockContext, 'test');
expect(result).toEqual([]);
});
it('should return matching hook names', () => {
const mockHooks: HookRegistryEntry[] = [
createMockHook('test-hook-1', HookEventName.BeforeTool, true),
createMockHook('test-hook-2', HookEventName.AfterTool, true),
createMockHook('other-hook', HookEventName.AfterAgent, false),
];
mockHookSystem.getAllHooks.mockReturnValue(mockHooks);
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.completion) {
throw new Error('enable command must have completion');
}
const result = enableCmd.completion(mockContext, 'test');
expect(result).toEqual(['test-hook-1', 'test-hook-2']);
});
it('should return all hook names when partial is empty', () => {
const mockHooks: HookRegistryEntry[] = [
createMockHook('hook-1', HookEventName.BeforeTool, true),
createMockHook('hook-2', HookEventName.AfterTool, true),
];
mockHookSystem.getAllHooks.mockReturnValue(mockHooks);
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.completion) {
throw new Error('enable command must have completion');
}
const result = enableCmd.completion(mockContext, '');
expect(result).toEqual(['hook-1', 'hook-2']);
});
it('should handle hooks without command name gracefully', () => {
const mockHooks: HookRegistryEntry[] = [
createMockHook('test-hook', HookEventName.BeforeTool, true),
{
...createMockHook('', HookEventName.AfterTool, true),
config: { command: '', type: HookType.Command, timeout: 30 },
},
];
mockHookSystem.getAllHooks.mockReturnValue(mockHooks);
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
);
if (!enableCmd?.completion) {
throw new Error('enable command must have completion');
}
const result = enableCmd.completion(mockContext, 'test');
expect(result).toEqual(['test-hook']);
});
});
});
/**
* Helper function to create a mock HookRegistryEntry
*/
function createMockHook(
command: string,
eventName: HookEventName,
enabled: boolean,
): HookRegistryEntry {
return {
config: {
command,
type: HookType.Command,
timeout: 30,
},
source: ConfigSource.Project,
eventName,
matcher: undefined,
sequential: false,
enabled,
};
}
@@ -0,0 +1,250 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import type {
SlashCommand,
CommandContext,
MessageActionReturn,
} from './types.js';
import { CommandKind } from './types.js';
import { MessageType, type HistoryItemHooksList } from '../types.js';
import type { HookRegistryEntry } from '@google/gemini-cli-core';
import { getErrorMessage } from '@google/gemini-cli-core';
import { SettingScope } from '../../config/settings.js';
/**
* Display a formatted list of hooks with their status
*/
async function panelAction(
context: CommandContext,
): Promise<void | MessageActionReturn> {
const { config } = context.services;
if (!config) {
return {
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
};
}
const hookSystem = config.getHookSystem();
if (!hookSystem) {
return {
type: 'message',
messageType: 'info',
content:
'Hook system is not enabled. Enable it in settings with tools.enableHooks',
};
}
const allHooks = hookSystem.getAllHooks();
if (allHooks.length === 0) {
return {
type: 'message',
messageType: 'info',
content:
'No hooks configured. Add hooks to your settings to get started.',
};
}
const hooksListItem: HistoryItemHooksList = {
type: MessageType.HOOKS_LIST,
hooks: allHooks,
};
context.ui.addItem(hooksListItem, Date.now());
}
/**
* Enable a hook by name
*/
async function enableAction(
context: CommandContext,
args: string,
): Promise<void | MessageActionReturn> {
const { config } = context.services;
if (!config) {
return {
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
};
}
const hookSystem = config.getHookSystem();
if (!hookSystem) {
return {
type: 'message',
messageType: 'error',
content: 'Hook system is not enabled.',
};
}
const hookName = args.trim();
if (!hookName) {
return {
type: 'message',
messageType: 'error',
content: 'Usage: /hooks enable <hook-name>',
};
}
// Get current disabled hooks from settings
const settings = context.services.settings;
const disabledHooks = settings.merged.hooks?.disabled || ([] as string[]);
// Remove from disabled list if present
const newDisabledHooks = disabledHooks.filter(
(name: string) => name !== hookName,
);
// Update settings (setValue automatically saves)
try {
settings.setValue(SettingScope.User, 'hooks.disabled', newDisabledHooks);
// Enable in hook system
hookSystem.setHookEnabled(hookName, true);
return {
type: 'message',
messageType: 'info',
content: `Hook "${hookName}" enabled successfully.`,
};
} catch (error) {
return {
type: 'message',
messageType: 'error',
content: `Failed to enable hook: ${getErrorMessage(error)}`,
};
}
}
/**
* Disable a hook by name
*/
async function disableAction(
context: CommandContext,
args: string,
): Promise<void | MessageActionReturn> {
const { config } = context.services;
if (!config) {
return {
type: 'message',
messageType: 'error',
content: 'Config not loaded.',
};
}
const hookSystem = config.getHookSystem();
if (!hookSystem) {
return {
type: 'message',
messageType: 'error',
content: 'Hook system is not enabled.',
};
}
const hookName = args.trim();
if (!hookName) {
return {
type: 'message',
messageType: 'error',
content: 'Usage: /hooks disable <hook-name>',
};
}
// Get current disabled hooks from settings
const settings = context.services.settings;
const disabledHooks = settings.merged.hooks?.disabled || ([] as string[]);
// Add to disabled list if not already present
if (!disabledHooks.includes(hookName)) {
const newDisabledHooks = [...disabledHooks, hookName];
// Update settings (setValue automatically saves)
try {
settings.setValue(SettingScope.User, 'hooks.disabled', newDisabledHooks);
// Disable in hook system
hookSystem.setHookEnabled(hookName, false);
return {
type: 'message',
messageType: 'info',
content: `Hook "${hookName}" disabled successfully.`,
};
} catch (error) {
return {
type: 'message',
messageType: 'error',
content: `Failed to disable hook: ${getErrorMessage(error)}`,
};
}
} else {
return {
type: 'message',
messageType: 'info',
content: `Hook "${hookName}" is already disabled.`,
};
}
}
/**
* Completion function for hook names
*/
function completeHookNames(
context: CommandContext,
partialArg: string,
): string[] {
const { config } = context.services;
if (!config) return [];
const hookSystem = config.getHookSystem();
if (!hookSystem) return [];
const allHooks = hookSystem.getAllHooks();
const hookNames = allHooks.map((hook) => getHookDisplayName(hook));
return hookNames.filter((name) => name.startsWith(partialArg));
}
/**
* Get a display name for a hook
*/
function getHookDisplayName(hook: HookRegistryEntry): string {
return hook.config.command || 'unknown-hook';
}
const panelCommand: SlashCommand = {
name: 'panel',
altNames: ['list', 'show'],
description: 'Display all registered hooks with their status',
kind: CommandKind.BUILT_IN,
action: panelAction,
};
const enableCommand: SlashCommand = {
name: 'enable',
description: 'Enable a hook by name',
kind: CommandKind.BUILT_IN,
action: enableAction,
completion: completeHookNames,
};
const disableCommand: SlashCommand = {
name: 'disable',
description: 'Disable a hook by name',
kind: CommandKind.BUILT_IN,
action: disableAction,
completion: completeHookNames,
};
export const hooksCommand: SlashCommand = {
name: 'hooks',
description: 'Manage hooks',
kind: CommandKind.BUILT_IN,
subCommands: [panelCommand, enableCommand, disableCommand],
action: async (context: CommandContext) => panelCommand.action!(context, ''),
};
@@ -30,6 +30,7 @@ import { getMCPServerStatus } from '@google/gemini-cli-core';
import { ToolsList } from './views/ToolsList.js';
import { McpStatus } from './views/McpStatus.js';
import { ChatList } from './views/ChatList.js';
import { HooksList } from './views/HooksList.js';
import { ModelMessage } from './messages/ModelMessage.js';
interface HistoryItemDisplayProps {
@@ -158,6 +159,9 @@ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({
{itemForDisplay.type === 'chat_list' && (
<ChatList chats={itemForDisplay.chats} />
)}
{itemForDisplay.type === 'hooks_list' && (
<HooksList hooks={itemForDisplay.hooks} />
)}
</Box>
);
};
@@ -0,0 +1,89 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import type React from 'react';
import { Box, Text } from 'ink';
interface HooksListProps {
hooks: ReadonlyArray<{
config: { command?: string; type: string; timeout?: number };
source: string;
eventName: string;
matcher?: string;
sequential?: boolean;
enabled: boolean;
}>;
}
export const HooksList: React.FC<HooksListProps> = ({ hooks }) => {
if (hooks.length === 0) {
return (
<Box marginTop={1} marginBottom={1}>
<Text>No hooks configured.</Text>
</Box>
);
}
// Group hooks by event name for better organization
const hooksByEvent = hooks.reduce(
(acc, hook) => {
if (!acc[hook.eventName]) {
acc[hook.eventName] = [];
}
acc[hook.eventName].push(hook);
return acc;
},
{} as Record<string, Array<(typeof hooks)[number]>>,
);
return (
<Box flexDirection="column" marginTop={1} marginBottom={1}>
<Text bold>Configured Hooks:</Text>
<Box flexDirection="column" paddingLeft={2} marginTop={1}>
{Object.entries(hooksByEvent).map(([eventName, eventHooks]) => (
<Box key={eventName} flexDirection="column" marginBottom={1}>
<Text color="cyan" bold>
{eventName}:
</Text>
<Box flexDirection="column" paddingLeft={2}>
{eventHooks.map((hook, index) => {
const hookName = hook.config.command || 'unknown';
const statusColor = hook.enabled ? 'green' : 'gray';
const statusText = hook.enabled ? 'enabled' : 'disabled';
return (
<Box key={`${eventName}-${index}`} flexDirection="column">
<Box>
<Text>
<Text color="yellow">{hookName}</Text>
<Text color={statusColor}>{` [${statusText}]`}</Text>
</Text>
</Box>
<Box paddingLeft={2} flexDirection="column">
<Text dimColor>
Source: {hook.source}
{hook.matcher && ` | Matcher: ${hook.matcher}`}
{hook.sequential && ` | Sequential`}
{hook.config.timeout &&
` | Timeout: ${hook.config.timeout}s`}
</Text>
</Box>
</Box>
);
})}
</Box>
</Box>
))}
</Box>
<Box marginTop={1}>
<Text dimColor>
Tip: Use `/hooks enable {'<hook-name>'}` or `/hooks disable{' '}
{'<hook-name>'}` to toggle hooks
</Text>
</Box>
</Box>
);
};
+15 -1
View File
@@ -240,6 +240,18 @@ export type HistoryItemMcpStatus = HistoryItemBase & {
showSchema: boolean;
};
export type HistoryItemHooksList = HistoryItemBase & {
type: 'hooks_list';
hooks: Array<{
config: { command?: string; type: string; timeout?: number };
source: string;
eventName: string;
matcher?: string;
sequential?: boolean;
enabled: boolean;
}>;
};
// Using Omit<HistoryItem, 'id'> seems to have some issues with typescript's
// type inference e.g. historyItem.type === 'tool_group' isn't auto-inferring that
// 'tools' in historyItem.
@@ -264,7 +276,8 @@ export type HistoryItemWithoutId =
| HistoryItemExtensionsList
| HistoryItemToolsList
| HistoryItemMcpStatus
| HistoryItemChatList;
| HistoryItemChatList
| HistoryItemHooksList;
export type HistoryItem = HistoryItemWithoutId & { id: number };
@@ -286,6 +299,7 @@ export enum MessageType {
TOOLS_LIST = 'tools_list',
MCP_STATUS = 'mcp_status',
CHAT_LIST = 'chat_list',
HOOKS_LIST = 'hooks_list',
}
// Simplified message structure for internal feedback