feat(hooks): add support for friendly names and descriptions (#15174)

This commit is contained in:
Abhi
2025-12-18 11:09:24 -05:00
committed by GitHub
parent 7da060c149
commit 54466a3ea8
14 changed files with 300 additions and 17 deletions
+20 -8
View File
@@ -414,13 +414,22 @@ precedence rules.
### Configuration layers ### Configuration layers
Hook configurations are applied in the following order of precedence (higher Hook configurations are applied in the following order of execution (lower
numbers override lower numbers): numbers run first):
1. **System defaults:** Built-in default settings (lowest precedence) 1. **Project settings:** `.gemini/settings.json` in your project directory
2. **User settings:** `~/.gemini/settings.json` (highest priority)
3. **Project settings:** `.gemini/settings.json` in your project directory 2. **User settings:** `~/.gemini/settings.json`
4. **System settings:** `/etc/gemini-cli/settings.json` (highest precedence) 3. **System settings:** `/etc/gemini-cli/settings.json`
4. **Extensions:** Internal hooks defined by installed extensions (lowest
priority)
#### Deduplication and shadowing
If multiple hooks with the identical **name** and **command** are discovered
across different configuration layers, Gemini CLI deduplicates them. The hook
from the higher-priority layer (e.g., Project) will be kept, and others will be
ignored.
Within each level, hooks run in the order they are declared in the Within each level, hooks run in the order they are declared in the
configuration. configuration.
@@ -450,8 +459,9 @@ configuration.
**Configuration properties:** **Configuration properties:**
- **`name`** (string, required): Unique identifier for the hook used in - **`name`** (string, recommended): Unique identifier for the hook used in
`/hooks enable/disable` commands `/hooks enable/disable` commands. If omitted, the `command` path is used as
the identifier.
- **`type`** (string, required): Hook type - currently only `"command"` is - **`type`** (string, required): Hook type - currently only `"command"` is
supported supported
- **`command`** (string, required): Path to the script or command to execute - **`command`** (string, required): Path to the script or command to execute
@@ -498,6 +508,8 @@ You can temporarily enable or disable individual hooks using commands:
These commands allow you to control hook execution without editing configuration These commands allow you to control hook execution without editing configuration
files. The hook name should match the `name` field in your hook configuration. files. The hook name should match the `name` field in your hook configuration.
Changes made via these commands are persisted to your global User settings
(`~/.gemini/settings.json`).
### Disabled hooks configuration ### Disabled hooks configuration
@@ -356,6 +356,18 @@ describe('SettingsSchema', () => {
'Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents', 'Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents',
); );
}); });
it('should have name and description in hook definitions', () => {
const hookDef = SETTINGS_SCHEMA_DEFINITIONS['HookDefinitionArray'];
expect(hookDef).toBeDefined();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const hookItemProperties = (hookDef as any).items.properties.hooks.items
.properties;
expect(hookItemProperties.name).toBeDefined();
expect(hookItemProperties.name.type).toBe('string');
expect(hookItemProperties.description).toBeDefined();
expect(hookItemProperties.description.type).toBe('string');
});
}); });
it('has JSON schema definitions for every referenced ref', () => { it('has JSON schema definitions for every referenced ref', () => {
@@ -1891,6 +1891,10 @@ export const SETTINGS_SCHEMA_DEFINITIONS: Record<
type: 'object', type: 'object',
description: 'Individual hook configuration.', description: 'Individual hook configuration.',
properties: { properties: {
name: {
type: 'string',
description: 'Unique identifier for the hook.',
},
type: { type: {
type: 'string', type: 'string',
description: description:
@@ -1901,6 +1905,10 @@ export const SETTINGS_SCHEMA_DEFINITIONS: Record<
description: description:
'Shell command to execute. Receives JSON input via stdin and returns JSON output via stdout.', 'Shell command to execute. Receives JSON input via stdin and returns JSON output via stdout.',
}, },
description: {
type: 'string',
description: 'A description of the hook.',
},
timeout: { timeout: {
type: 'number', type: 'number',
description: 'Timeout in milliseconds for hook execution.', description: 'Timeout in milliseconds for hook execution.',
@@ -309,6 +309,24 @@ describe('hooksCommand', () => {
content: 'Failed to enable hook: Failed to save settings', content: 'Failed to enable hook: Failed to save settings',
}); });
}); });
it('should complete hook names using friendly names', () => {
const enableCmd = hooksCommand.subCommands!.find(
(cmd) => cmd.name === 'enable',
)!;
const hookEntry = createMockHook(
'./hooks/test.sh',
HookEventName.BeforeTool,
true,
);
hookEntry.config.name = 'friendly-name';
mockHookSystem.getAllHooks.mockReturnValue([hookEntry]);
const completions = enableCmd.completion!(mockContext, 'frie');
expect(completions).toContain('friendly-name');
});
}); });
describe('disable subcommand', () => { describe('disable subcommand', () => {
+1 -1
View File
@@ -213,7 +213,7 @@ function completeHookNames(
* Get a display name for a hook * Get a display name for a hook
*/ */
function getHookDisplayName(hook: HookRegistryEntry): string { function getHookDisplayName(hook: HookRegistryEntry): string {
return hook.config.command || 'unknown-hook'; return hook.config.name || hook.config.command || 'unknown-hook';
} }
const panelCommand: SlashCommand = { const panelCommand: SlashCommand = {
@@ -9,7 +9,13 @@ import { Box, Text } from 'ink';
interface HooksListProps { interface HooksListProps {
hooks: ReadonlyArray<{ hooks: ReadonlyArray<{
config: { command?: string; type: string; timeout?: number }; config: {
command?: string;
type: string;
name?: string;
description?: string;
timeout?: number;
};
source: string; source: string;
eventName: string; eventName: string;
matcher?: string; matcher?: string;
@@ -50,7 +56,8 @@ export const HooksList: React.FC<HooksListProps> = ({ hooks }) => {
</Text> </Text>
<Box flexDirection="column" paddingLeft={2}> <Box flexDirection="column" paddingLeft={2}>
{eventHooks.map((hook, index) => { {eventHooks.map((hook, index) => {
const hookName = hook.config.command || 'unknown'; const hookName =
hook.config.name || hook.config.command || 'unknown';
const statusColor = hook.enabled ? 'green' : 'gray'; const statusColor = hook.enabled ? 'green' : 'gray';
const statusText = hook.enabled ? 'enabled' : 'disabled'; const statusText = hook.enabled ? 'enabled' : 'disabled';
@@ -63,8 +70,14 @@ export const HooksList: React.FC<HooksListProps> = ({ hooks }) => {
</Text> </Text>
</Box> </Box>
<Box paddingLeft={2} flexDirection="column"> <Box paddingLeft={2} flexDirection="column">
{hook.config.description && (
<Text italic>{hook.config.description}</Text>
)}
<Text dimColor> <Text dimColor>
Source: {hook.source} Source: {hook.source}
{hook.config.name &&
hook.config.command &&
` | Command: ${hook.config.command}`}
{hook.matcher && ` | Matcher: ${hook.matcher}`} {hook.matcher && ` | Matcher: ${hook.matcher}`}
{hook.sequential && ` | Sequential`} {hook.sequential && ` | Sequential`}
{hook.config.timeout && {hook.config.timeout &&
@@ -282,6 +282,71 @@ describe('HookPlanner', () => {
); );
}); });
it('should deduplicate based on both name and command', () => {
const mockEntries: HookRegistryEntry[] = [
{
config: {
name: 'hook1',
type: HookType.Command,
command: './same.sh',
},
source: ConfigSource.Project,
eventName: HookEventName.BeforeTool,
enabled: true,
},
{
config: {
name: 'hook1',
type: HookType.Command,
command: './same.sh',
},
source: ConfigSource.User,
eventName: HookEventName.BeforeTool,
enabled: true,
}, // Same name, same command -> deduplicate
{
config: {
name: 'hook2',
type: HookType.Command,
command: './same.sh',
},
source: ConfigSource.Project,
eventName: HookEventName.BeforeTool,
enabled: true,
}, // Different name, same command -> distinct
{
config: {
name: 'hook1',
type: HookType.Command,
command: './different.sh',
},
source: ConfigSource.Project,
eventName: HookEventName.BeforeTool,
enabled: true,
}, // Same name, different command -> distinct
{
config: { type: HookType.Command, command: './no-name.sh' },
source: ConfigSource.Project,
eventName: HookEventName.BeforeTool,
enabled: true,
},
{
config: { type: HookType.Command, command: './no-name.sh' },
source: ConfigSource.User,
eventName: HookEventName.BeforeTool,
enabled: true,
}, // No name, same command -> deduplicate
];
vi.mocked(mockHookRegistry.getHooksForEvent).mockReturnValue(mockEntries);
const plan = hookPlanner.createExecutionPlan(HookEventName.BeforeTool);
expect(plan).not.toBeNull();
// hook1:same.sh (deduped), hook2:same.sh, hook1:different.sh, :no-name.sh (deduped)
expect(plan!.hookConfigs).toHaveLength(4);
});
it('should match trigger for session events', () => { it('should match trigger for session events', () => {
const mockEntries: HookRegistryEntry[] = [ const mockEntries: HookRegistryEntry[] = [
{ {
+3 -1
View File
@@ -141,7 +141,9 @@ export class HookPlanner {
* Generate a unique key for a hook entry * Generate a unique key for a hook entry
*/ */
private getHookKey(entry: HookRegistryEntry): string { private getHookKey(entry: HookRegistryEntry): string {
return `command:${entry.config.command}`; const name = entry.config.name || '';
const command = entry.config.command || '';
return `${name}:${command}`;
} }
} }
@@ -190,6 +190,39 @@ describe('HookRegistry', () => {
expect(hookRegistry.getAllHooks()).toHaveLength(0); expect(hookRegistry.getAllHooks()).toHaveLength(0);
expect(mockDebugLogger.warn).toHaveBeenCalled(); // At least some warnings should be logged expect(mockDebugLogger.warn).toHaveBeenCalled(); // At least some warnings should be logged
}); });
it('should respect disabled hooks using friendly name', async () => {
const mockHooksConfig = {
BeforeTool: [
{
hooks: [
{
name: 'disabled-hook',
type: 'command',
command: './hooks/test.sh',
},
],
},
],
};
// Update mock to return the hooks configuration
vi.mocked(mockConfig.getHooks).mockReturnValue(
mockHooksConfig as unknown as {
[K in HookEventName]?: HookDefinition[];
},
);
vi.mocked(mockConfig.getDisabledHooks).mockReturnValue(['disabled-hook']);
await hookRegistry.initialize();
const hooks = hookRegistry.getAllHooks();
expect(hooks).toHaveLength(1);
expect(hooks[0].enabled).toBe(false);
expect(
hookRegistry.getHooksForEvent(HookEventName.BeforeTool),
).toHaveLength(0);
});
}); });
describe('getHooksForEvent', () => { describe('getHooksForEvent', () => {
@@ -305,6 +338,77 @@ describe('HookRegistry', () => {
'No hooks found matching "non-existent-hook"', 'No hooks found matching "non-existent-hook"',
); );
}); });
it('should prefer hook name over command for identification', async () => {
const mockHooksConfig = {
BeforeTool: [
{
hooks: [
{
name: 'friendly-name',
type: 'command',
command: './hooks/test.sh',
},
],
},
],
};
vi.mocked(mockConfig.getHooks).mockReturnValue(
mockHooksConfig as unknown as {
[K in HookEventName]?: HookDefinition[];
},
);
await hookRegistry.initialize();
// Should be enabled initially
let hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
expect(hooks).toHaveLength(1);
// Disable using friendly name
hookRegistry.setHookEnabled('friendly-name', false);
hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
expect(hooks).toHaveLength(0);
// Identification by command should NOT work when name is present
hookRegistry.setHookEnabled('./hooks/test.sh', true);
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
'No hooks found matching "./hooks/test.sh"',
);
});
it('should use command as identifier when name is missing', async () => {
const mockHooksConfig = {
BeforeTool: [
{
hooks: [
{
type: 'command',
command: './hooks/no-name.sh',
},
],
},
],
};
vi.mocked(mockConfig.getHooks).mockReturnValue(
mockHooksConfig as unknown as {
[K in HookEventName]?: HookDefinition[];
},
);
await hookRegistry.initialize();
// Should be enabled initially
let hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
expect(hooks).toHaveLength(1);
// Disable using command
hookRegistry.setHookEnabled('./hooks/no-name.sh', false);
hooks = hookRegistry.getHooksForEvent(HookEventName.BeforeTool);
expect(hooks).toHaveLength(0);
});
}); });
describe('malformed configuration handling', () => { describe('malformed configuration handling', () => {
+5 -3
View File
@@ -96,10 +96,12 @@ export class HookRegistry {
} }
/** /**
* Get hook name for display purposes * Get hook name for identification and display purposes
*/ */
private getHookName(entry: HookRegistryEntry): string { private getHookName(
return entry.config.command || 'unknown-command'; entry: HookRegistryEntry | { config: HookConfig },
): string {
return entry.config.name || entry.config.command || 'unknown-command';
} }
/** /**
@@ -28,6 +28,18 @@ vi.mock('node:child_process', async (importOriginal) => {
}; };
}); });
// Mock debugLogger using vi.hoisted
const mockDebugLogger = vi.hoisted(() => ({
log: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
}));
vi.mock('../utils/debugLogger.js', () => ({
debugLogger: mockDebugLogger,
}));
// Mock console methods // Mock console methods
const mockConsole = { const mockConsole = {
log: vi.fn(), log: vi.fn(),
@@ -162,6 +174,31 @@ describe('HookRunner', () => {
expect(result.stderr).toBe(errorMessage); expect(result.stderr).toBe(errorMessage);
}); });
it('should use hook name in error messages if available', async () => {
const namedConfig: HookConfig = {
name: 'my-friendly-hook',
type: HookType.Command,
command: './hooks/fail.sh',
};
// Mock error during spawn
vi.mocked(spawn).mockImplementationOnce(() => {
throw new Error('Spawn error');
});
await hookRunner.executeHook(
namedConfig,
HookEventName.BeforeTool,
mockInput,
);
expect(mockDebugLogger.warn).toHaveBeenCalledWith(
expect.stringContaining(
'(hook: my-friendly-hook): Error: Spawn error',
),
);
});
it('should handle command hook timeout', async () => { it('should handle command hook timeout', async () => {
const shortTimeoutConfig: HookConfig = { const shortTimeoutConfig: HookConfig = {
type: HookType.Command, type: HookType.Command,
+2 -2
View File
@@ -55,8 +55,8 @@ export class HookRunner {
); );
} catch (error) { } catch (error) {
const duration = Date.now() - startTime; const duration = Date.now() - startTime;
const hookSource = hookConfig.command || 'unknown'; const hookId = hookConfig.name || hookConfig.command || 'unknown';
const errorMessage = `Hook execution failed for event '${eventName}' (source: ${hookSource}): ${error}`; const errorMessage = `Hook execution failed for event '${eventName}' (hook: ${hookId}): ${error}`;
debugLogger.warn(`Hook execution error (non-fatal): ${errorMessage}`); debugLogger.warn(`Hook execution error (non-fatal): ${errorMessage}`);
return { return {
+2
View File
@@ -40,6 +40,8 @@ export enum HookEventName {
export interface CommandHookConfig { export interface CommandHookConfig {
type: HookType.Command; type: HookType.Command;
command: string; command: string;
name?: string;
description?: string;
timeout?: number; timeout?: number;
} }
+8
View File
@@ -1867,6 +1867,10 @@
"type": "object", "type": "object",
"description": "Individual hook configuration.", "description": "Individual hook configuration.",
"properties": { "properties": {
"name": {
"type": "string",
"description": "Unique identifier for the hook."
},
"type": { "type": {
"type": "string", "type": "string",
"description": "Type of hook (currently only \"command\" supported)." "description": "Type of hook (currently only \"command\" supported)."
@@ -1875,6 +1879,10 @@
"type": "string", "type": "string",
"description": "Shell command to execute. Receives JSON input via stdin and returns JSON output via stdout." "description": "Shell command to execute. Receives JSON input via stdin and returns JSON output via stdout."
}, },
"description": {
"type": "string",
"description": "A description of the hook."
},
"timeout": { "timeout": {
"type": "number", "type": "number",
"description": "Timeout in milliseconds for hook execution." "description": "Timeout in milliseconds for hook execution."