From 4dce98ae1de9fb4d308673e5025ddfd958cd26e2 Mon Sep 17 00:00:00 2001 From: qqqys Date: Fri, 11 Sep 2026 12:11:20 +0800 Subject: [PATCH 1/3] chore(core): cover every hook event in the settings schema and hook bus The settings schema listed 17 of the 22 hook events the runtime accepts, so PostCompact, PermissionDenied, TodoCreated, TodoCompleted and InstructionsLoaded looked like unknown keys in editors. The hook execution bus handled 14 events and logged "Unknown hook event" for the rest. Add the five schema entries and regenerate the JSON schema, with a test that the schema's hook events equal the HookEventName enum. Add bus cases for SessionStart, SessionEnd, SessionDelete, PreCompact, PostCompact, InstructionsLoaded, StopFailure, TodoCreated and TodoCompleted; the last three return aggregated results, so the bus replies with their final output like every other event. Part of #11610 --- .../cli/src/config/settingsSchema.test.ts | 7 + packages/cli/src/config/settingsSchema.ts | 60 ++ packages/core/src/config/config.test.ts | 145 +++++ packages/core/src/config/config.ts | 97 +++ .../schemas/settings.schema.json | 560 ++++++++++++++++++ 5 files changed, 869 insertions(+) diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 19441296560..0bdd16a3fc8 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -11,6 +11,7 @@ import { GOAL_MAX_ACTIVE_MINUTES_CAP, GOAL_MAX_TURNS_CAP, HELD_EXPIRY_OPTIONS, + HookEventName, DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH, OutputFormat, SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH_LIMIT, @@ -45,6 +46,12 @@ describe('SettingsSchema', () => { expect(hookProperties?.['model']).toMatchObject({ type: 'string' }); }); + it('should declare a hooks setting for every hook event', () => { + expect( + Object.keys(getSettingsSchema().hooks.properties ?? {}).sort(), + ).toEqual([...Object.values(HookEventName)].sort()); + }); + it('should contain all expected top-level settings', () => { const expectedSettings: Array = [ 'mcpServers', diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 7d2f9c6cbf6..cf9a8287f14 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -3880,6 +3880,66 @@ const SETTINGS_SCHEMA = { mergeStrategy: MergeStrategy.CONCAT, items: HOOK_DEFINITION_ITEMS, }, + PostCompact: { + type: 'array', + label: 'Post Compact Hooks', + category: 'Advanced', + requiresRestart: false, + default: [], + description: + 'Hooks that execute after conversation compaction completes.', + showInDialog: false, + mergeStrategy: MergeStrategy.CONCAT, + items: HOOK_DEFINITION_ITEMS, + }, + PermissionDenied: { + type: 'array', + label: 'Permission Denied Hooks', + category: 'Advanced', + requiresRestart: false, + default: [], + description: + 'Hooks that execute when AUTO-mode classification denies a tool call.', + showInDialog: false, + mergeStrategy: MergeStrategy.CONCAT, + items: HOOK_DEFINITION_ITEMS, + }, + TodoCreated: { + type: 'array', + label: 'Todo Created Hooks', + category: 'Advanced', + requiresRestart: false, + default: [], + description: + 'Hooks that execute when a new todo item is created. They can block creation during validation.', + showInDialog: false, + mergeStrategy: MergeStrategy.CONCAT, + items: HOOK_DEFINITION_ITEMS, + }, + TodoCompleted: { + type: 'array', + label: 'Todo Completed Hooks', + category: 'Advanced', + requiresRestart: false, + default: [], + description: + 'Hooks that execute when a todo item is marked as completed. They can block completion during validation.', + showInDialog: false, + mergeStrategy: MergeStrategy.CONCAT, + items: HOOK_DEFINITION_ITEMS, + }, + InstructionsLoaded: { + type: 'array', + label: 'Instructions Loaded Hooks', + category: 'Advanced', + requiresRestart: false, + default: [], + description: + 'Hooks that execute when an instruction file such as QWEN.md is loaded into context.', + showInDialog: false, + mergeStrategy: MergeStrategy.CONCAT, + items: HOOK_DEFINITION_ITEMS, + }, }, }, diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index d671822438a..3eb8a6d6eb9 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -13673,6 +13673,151 @@ describe('Model Switching and Config Updates', () => { }); }); + describe('direct-call hook events through the hook execution bridge', () => { + const dispatch = async ( + method: string, + fire: ReturnType, + eventName: string, + input: Record, + ) => { + const config = new Config({ ...baseParams }); + await config.initialize(); + // @ts-expect-error - accessing private for testing + config['hookSystem'] = { [method]: fire }; + return config + .getMessageBus()! + .request< + HookExecutionRequest, + HookExecutionResponse + >({ type: MessageBusType.HOOK_EXECUTION_REQUEST, eventName, input }, MessageBusType.HOOK_EXECUTION_RESPONSE); + }; + + it.each([ + { + eventName: 'SessionStart', + method: 'fireSessionStartEvent', + input: { + source: 'resume', + model: 'qwen-max', + permission_mode: 'plan', + agent_type: 'general-purpose', + }, + args: ['resume', 'qwen-max', 'plan', 'general-purpose'], + }, + { + eventName: 'SessionEnd', + method: 'fireSessionEndEvent', + input: { reason: 'clear' }, + args: ['clear'], + }, + { + eventName: 'SessionDelete', + method: 'fireSessionDeleteEvent', + input: { deleted_session_id: 'old-session' }, + args: ['old-session'], + }, + { + eventName: 'PreCompact', + method: 'firePreCompactEvent', + input: { trigger: 'manual', custom_instructions: 'keep todos' }, + args: ['manual', 'keep todos'], + }, + { + eventName: 'PostCompact', + method: 'firePostCompactEvent', + input: { trigger: 'auto', compact_summary: 'summary' }, + args: ['auto', 'summary'], + }, + { + eventName: 'InstructionsLoaded', + method: 'fireInstructionsLoadedEvent', + input: { + file_path: '/repo/QWEN.md', + memory_type: 'project', + load_reason: 'session_start', + trigger_file_path: '/repo/src/a.ts', + parent_file_path: '/repo/QWEN.md', + }, + args: [ + '/repo/QWEN.md', + 'project', + 'session_start', + { + triggerFilePath: '/repo/src/a.ts', + parentFilePath: '/repo/QWEN.md', + }, + ], + }, + ])( + 'forwards $eventName to $method', + async ({ eventName, method, input, args }) => { + const output = { systemMessage: `${eventName} ran` }; + const fire = vi.fn().mockResolvedValue(output); + + const response = await dispatch(method, fire, eventName, input); + + expect(fire).toHaveBeenCalledWith(...args, undefined); + expect(response.success).toBe(true); + expect(response.output).toEqual(output); + }, + ); + + it.each([ + { + eventName: 'StopFailure', + method: 'fireStopFailureEvent', + input: { + error: 'rate_limit', + error_details: '429 Too Many Requests', + last_assistant_message: 'partial', + }, + args: ['rate_limit', '429 Too Many Requests', 'partial'], + }, + { + eventName: 'TodoCreated', + method: 'fireTodoCreatedEvent', + input: { + todo_id: '1', + todo_content: 'write tests', + todo_status: 'pending', + all_todos: [], + phase: 'validation', + }, + args: ['1', 'write tests', 'pending', [], 'validation'], + }, + { + eventName: 'TodoCompleted', + method: 'fireTodoCompletedEvent', + input: { + todo_id: '1', + todo_content: 'write tests', + previous_status: 'in_progress', + all_todos: [], + phase: 'postWrite', + }, + args: ['1', 'write tests', 'in_progress', [], 'postWrite'], + }, + ])( + 'forwards $eventName to $method and returns its final output', + async ({ eventName, method, input, args }) => { + const finalOutput = { decision: 'block', reason: 'not yet' }; + const fire = vi.fn().mockResolvedValue({ + success: true, + allOutputs: [finalOutput], + errors: [], + totalDuration: 1, + finalOutput, + }); + + const response = await dispatch(method, fire, eventName, input); + + expect(fire).toHaveBeenCalledWith(...args, undefined); + expect(response.success).toBe(true); + expect(response.output).toEqual(finalOutput); + }, + ); + }); + describe('Stop dispatch through the hook execution bridge', () => { // The goal-specific half of this suite went with the two response fields // it asserted. What remains is the only exercise of the surviving diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b1896db3720..3d685b09df6 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -176,6 +176,16 @@ import { type HookEventName, type HookDefinition, type PostToolBatchToolCall, + type AgentType, + type HookPhase, + type InstructionMemoryType, + type PostCompactTrigger, + type PreCompactTrigger, + type SessionEndReason, + type SessionStartSource, + type StopFailureErrorType, + type TodoItem, + type TodoStatus, } from '../hooks/types.js'; import { fireNotificationHook } from '../core/toolHookTriggers.js'; import { @@ -3561,6 +3571,93 @@ export class Config { signal, ); break; + case 'SessionStart': + result = await hookSystem.fireSessionStartEvent( + input['source'] as SessionStartSource, + (input['model'] as string) || '', + (input['permission_mode'] as PermissionMode) || undefined, + input['agent_type'] as AgentType | undefined, + signal, + ); + break; + case 'SessionEnd': + result = await hookSystem.fireSessionEndEvent( + input['reason'] as SessionEndReason, + signal, + ); + break; + case 'SessionDelete': + result = await hookSystem.fireSessionDeleteEvent( + (input['deleted_session_id'] as string) || '', + signal, + ); + break; + case 'PreCompact': + result = await hookSystem.firePreCompactEvent( + input['trigger'] as PreCompactTrigger, + (input['custom_instructions'] as string) || '', + signal, + ); + break; + case 'PostCompact': + result = await hookSystem.firePostCompactEvent( + input['trigger'] as PostCompactTrigger, + (input['compact_summary'] as string) || '', + signal, + ); + break; + case 'InstructionsLoaded': + result = await hookSystem.fireInstructionsLoadedEvent( + (input['file_path'] as string) || '', + input['memory_type'] as InstructionMemoryType, + input['load_reason'] as InstructionLoadReason, + { + triggerFilePath: input['trigger_file_path'] as + | string + | undefined, + parentFilePath: input['parent_file_path'] as + | string + | undefined, + }, + signal, + ); + break; + // These three return the aggregated result; the bus carries its + // final output like every other event. + case 'StopFailure': + result = ( + await hookSystem.fireStopFailureEvent( + input['error'] as StopFailureErrorType, + input['error_details'] as string | undefined, + input['last_assistant_message'] as string | undefined, + signal, + ) + ).finalOutput; + break; + case 'TodoCreated': + result = ( + await hookSystem.fireTodoCreatedEvent( + (input['todo_id'] as string) || '', + (input['todo_content'] as string) || '', + input['todo_status'] as TodoStatus, + (input['all_todos'] as TodoItem[]) || [], + input['phase'] as HookPhase, + signal, + ) + ).finalOutput; + break; + case 'TodoCompleted': + result = ( + await hookSystem.fireTodoCompletedEvent( + (input['todo_id'] as string) || '', + (input['todo_content'] as string) || '', + input['previous_status'] as 'pending' | 'in_progress', + (input['all_todos'] as TodoItem[]) || [], + input['phase'] as HookPhase, + signal, + ) + ).finalOutput; + break; default: this.debugLogger.warn( `Unknown hook event: ${request.eventName}`, diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index d9a49cf0957..8af007f76f9 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -3668,6 +3668,566 @@ "hooks" ] } + }, + "PostCompact": { + "description": "Hooks that execute after conversation compaction completes.", + "type": "array", + "items": { + "description": "A hook definition with an optional matcher and a list of hook configurations.", + "type": "object", + "properties": { + "matcher": { + "description": "An optional matcher pattern to filter when this hook definition applies.", + "type": "string" + }, + "sequential": { + "description": "Whether the hooks should be executed sequentially instead of in parallel.", + "type": "boolean" + }, + "hooks": { + "description": "The list of hook configurations to execute.", + "type": "array", + "items": { + "description": "A hook configuration entry that defines a hook to execute.", + "type": "object", + "properties": { + "type": { + "description": "The type of hook. Note: \"function\" type is only available via SDK registration, not settings.json.", + "type": "string", + "enum": [ + "command", + "http", + "prompt" + ] + }, + "command": { + "description": "The command to execute when the hook is triggered. Required for \"command\" type.", + "type": "string" + }, + "url": { + "description": "The URL to send the POST request to. Required for \"http\" type.", + "type": "string" + }, + "prompt": { + "description": "The prompt to send to the model. Required for \"prompt\" type.", + "type": "string" + }, + "model": { + "description": "The optional model to use for a \"prompt\" hook.", + "type": "string" + }, + "headers": { + "description": "HTTP headers to include in the request. Supports env var interpolation ($VAR, ${VAR}).", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "description": "List of environment variables allowed for interpolation in headers and URL.", + "type": "array", + "items": { + "type": "string" + } + }, + "name": { + "description": "An optional name for the hook.", + "type": "string" + }, + "description": { + "description": "An optional description of what the hook does.", + "type": "string" + }, + "timeout": { + "description": "Timeout in seconds for the hook execution.", + "type": "number" + }, + "env": { + "description": "Environment variables to set when executing the hook command.", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "async": { + "description": "Whether to execute the hook asynchronously (non-blocking, for \"command\" type only).", + "type": "boolean" + }, + "once": { + "description": "Whether to execute the hook only once per session (for \"http\" type).", + "type": "boolean" + }, + "statusMessage": { + "description": "A message to display while the hook is executing.", + "type": "string" + }, + "shell": { + "description": "The shell to use for command execution.", + "type": "string", + "enum": [ + "bash", + "powershell" + ] + } + }, + "required": [ + "type" + ] + } + } + }, + "required": [ + "hooks" + ] + } + }, + "PermissionDenied": { + "description": "Hooks that execute when AUTO-mode classification denies a tool call.", + "type": "array", + "items": { + "description": "A hook definition with an optional matcher and a list of hook configurations.", + "type": "object", + "properties": { + "matcher": { + "description": "An optional matcher pattern to filter when this hook definition applies.", + "type": "string" + }, + "sequential": { + "description": "Whether the hooks should be executed sequentially instead of in parallel.", + "type": "boolean" + }, + "hooks": { + "description": "The list of hook configurations to execute.", + "type": "array", + "items": { + "description": "A hook configuration entry that defines a hook to execute.", + "type": "object", + "properties": { + "type": { + "description": "The type of hook. Note: \"function\" type is only available via SDK registration, not settings.json.", + "type": "string", + "enum": [ + "command", + "http", + "prompt" + ] + }, + "command": { + "description": "The command to execute when the hook is triggered. Required for \"command\" type.", + "type": "string" + }, + "url": { + "description": "The URL to send the POST request to. Required for \"http\" type.", + "type": "string" + }, + "prompt": { + "description": "The prompt to send to the model. Required for \"prompt\" type.", + "type": "string" + }, + "model": { + "description": "The optional model to use for a \"prompt\" hook.", + "type": "string" + }, + "headers": { + "description": "HTTP headers to include in the request. Supports env var interpolation ($VAR, ${VAR}).", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "description": "List of environment variables allowed for interpolation in headers and URL.", + "type": "array", + "items": { + "type": "string" + } + }, + "name": { + "description": "An optional name for the hook.", + "type": "string" + }, + "description": { + "description": "An optional description of what the hook does.", + "type": "string" + }, + "timeout": { + "description": "Timeout in seconds for the hook execution.", + "type": "number" + }, + "env": { + "description": "Environment variables to set when executing the hook command.", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "async": { + "description": "Whether to execute the hook asynchronously (non-blocking, for \"command\" type only).", + "type": "boolean" + }, + "once": { + "description": "Whether to execute the hook only once per session (for \"http\" type).", + "type": "boolean" + }, + "statusMessage": { + "description": "A message to display while the hook is executing.", + "type": "string" + }, + "shell": { + "description": "The shell to use for command execution.", + "type": "string", + "enum": [ + "bash", + "powershell" + ] + } + }, + "required": [ + "type" + ] + } + } + }, + "required": [ + "hooks" + ] + } + }, + "TodoCreated": { + "description": "Hooks that execute when a new todo item is created. They can block creation during validation.", + "type": "array", + "items": { + "description": "A hook definition with an optional matcher and a list of hook configurations.", + "type": "object", + "properties": { + "matcher": { + "description": "An optional matcher pattern to filter when this hook definition applies.", + "type": "string" + }, + "sequential": { + "description": "Whether the hooks should be executed sequentially instead of in parallel.", + "type": "boolean" + }, + "hooks": { + "description": "The list of hook configurations to execute.", + "type": "array", + "items": { + "description": "A hook configuration entry that defines a hook to execute.", + "type": "object", + "properties": { + "type": { + "description": "The type of hook. Note: \"function\" type is only available via SDK registration, not settings.json.", + "type": "string", + "enum": [ + "command", + "http", + "prompt" + ] + }, + "command": { + "description": "The command to execute when the hook is triggered. Required for \"command\" type.", + "type": "string" + }, + "url": { + "description": "The URL to send the POST request to. Required for \"http\" type.", + "type": "string" + }, + "prompt": { + "description": "The prompt to send to the model. Required for \"prompt\" type.", + "type": "string" + }, + "model": { + "description": "The optional model to use for a \"prompt\" hook.", + "type": "string" + }, + "headers": { + "description": "HTTP headers to include in the request. Supports env var interpolation ($VAR, ${VAR}).", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "description": "List of environment variables allowed for interpolation in headers and URL.", + "type": "array", + "items": { + "type": "string" + } + }, + "name": { + "description": "An optional name for the hook.", + "type": "string" + }, + "description": { + "description": "An optional description of what the hook does.", + "type": "string" + }, + "timeout": { + "description": "Timeout in seconds for the hook execution.", + "type": "number" + }, + "env": { + "description": "Environment variables to set when executing the hook command.", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "async": { + "description": "Whether to execute the hook asynchronously (non-blocking, for \"command\" type only).", + "type": "boolean" + }, + "once": { + "description": "Whether to execute the hook only once per session (for \"http\" type).", + "type": "boolean" + }, + "statusMessage": { + "description": "A message to display while the hook is executing.", + "type": "string" + }, + "shell": { + "description": "The shell to use for command execution.", + "type": "string", + "enum": [ + "bash", + "powershell" + ] + } + }, + "required": [ + "type" + ] + } + } + }, + "required": [ + "hooks" + ] + } + }, + "TodoCompleted": { + "description": "Hooks that execute when a todo item is marked as completed. They can block completion during validation.", + "type": "array", + "items": { + "description": "A hook definition with an optional matcher and a list of hook configurations.", + "type": "object", + "properties": { + "matcher": { + "description": "An optional matcher pattern to filter when this hook definition applies.", + "type": "string" + }, + "sequential": { + "description": "Whether the hooks should be executed sequentially instead of in parallel.", + "type": "boolean" + }, + "hooks": { + "description": "The list of hook configurations to execute.", + "type": "array", + "items": { + "description": "A hook configuration entry that defines a hook to execute.", + "type": "object", + "properties": { + "type": { + "description": "The type of hook. Note: \"function\" type is only available via SDK registration, not settings.json.", + "type": "string", + "enum": [ + "command", + "http", + "prompt" + ] + }, + "command": { + "description": "The command to execute when the hook is triggered. Required for \"command\" type.", + "type": "string" + }, + "url": { + "description": "The URL to send the POST request to. Required for \"http\" type.", + "type": "string" + }, + "prompt": { + "description": "The prompt to send to the model. Required for \"prompt\" type.", + "type": "string" + }, + "model": { + "description": "The optional model to use for a \"prompt\" hook.", + "type": "string" + }, + "headers": { + "description": "HTTP headers to include in the request. Supports env var interpolation ($VAR, ${VAR}).", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "description": "List of environment variables allowed for interpolation in headers and URL.", + "type": "array", + "items": { + "type": "string" + } + }, + "name": { + "description": "An optional name for the hook.", + "type": "string" + }, + "description": { + "description": "An optional description of what the hook does.", + "type": "string" + }, + "timeout": { + "description": "Timeout in seconds for the hook execution.", + "type": "number" + }, + "env": { + "description": "Environment variables to set when executing the hook command.", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "async": { + "description": "Whether to execute the hook asynchronously (non-blocking, for \"command\" type only).", + "type": "boolean" + }, + "once": { + "description": "Whether to execute the hook only once per session (for \"http\" type).", + "type": "boolean" + }, + "statusMessage": { + "description": "A message to display while the hook is executing.", + "type": "string" + }, + "shell": { + "description": "The shell to use for command execution.", + "type": "string", + "enum": [ + "bash", + "powershell" + ] + } + }, + "required": [ + "type" + ] + } + } + }, + "required": [ + "hooks" + ] + } + }, + "InstructionsLoaded": { + "description": "Hooks that execute when an instruction file such as QWEN.md is loaded into context.", + "type": "array", + "items": { + "description": "A hook definition with an optional matcher and a list of hook configurations.", + "type": "object", + "properties": { + "matcher": { + "description": "An optional matcher pattern to filter when this hook definition applies.", + "type": "string" + }, + "sequential": { + "description": "Whether the hooks should be executed sequentially instead of in parallel.", + "type": "boolean" + }, + "hooks": { + "description": "The list of hook configurations to execute.", + "type": "array", + "items": { + "description": "A hook configuration entry that defines a hook to execute.", + "type": "object", + "properties": { + "type": { + "description": "The type of hook. Note: \"function\" type is only available via SDK registration, not settings.json.", + "type": "string", + "enum": [ + "command", + "http", + "prompt" + ] + }, + "command": { + "description": "The command to execute when the hook is triggered. Required for \"command\" type.", + "type": "string" + }, + "url": { + "description": "The URL to send the POST request to. Required for \"http\" type.", + "type": "string" + }, + "prompt": { + "description": "The prompt to send to the model. Required for \"prompt\" type.", + "type": "string" + }, + "model": { + "description": "The optional model to use for a \"prompt\" hook.", + "type": "string" + }, + "headers": { + "description": "HTTP headers to include in the request. Supports env var interpolation ($VAR, ${VAR}).", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "description": "List of environment variables allowed for interpolation in headers and URL.", + "type": "array", + "items": { + "type": "string" + } + }, + "name": { + "description": "An optional name for the hook.", + "type": "string" + }, + "description": { + "description": "An optional description of what the hook does.", + "type": "string" + }, + "timeout": { + "description": "Timeout in seconds for the hook execution.", + "type": "number" + }, + "env": { + "description": "Environment variables to set when executing the hook command.", + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "async": { + "description": "Whether to execute the hook asynchronously (non-blocking, for \"command\" type only).", + "type": "boolean" + }, + "once": { + "description": "Whether to execute the hook only once per session (for \"http\" type).", + "type": "boolean" + }, + "statusMessage": { + "description": "A message to display while the hook is executing.", + "type": "string" + }, + "shell": { + "description": "The shell to use for command execution.", + "type": "string", + "enum": [ + "bash", + "powershell" + ] + } + }, + "required": [ + "type" + ] + } + } + }, + "required": [ + "hooks" + ] + } } } }, From 0363b80e9bf3e345de61b7cc0ec298f797f8a4bb Mon Sep 17 00:00:00 2001 From: qqqys Date: Fri, 11 Sep 2026 16:27:05 +0800 Subject: [PATCH 2/3] test(cli): pin that hook definitions concatenate across settings scopes The five hook events this PR adds to the settings schema also gain the CONCAT merge strategy, so a definition in workspace settings no longer replaces the user's for PostCompact, PermissionDenied, TodoCreated, TodoCompleted and InstructionsLoaded. Pin that with a loadSettings merge test and a schema check that every hook event concatenates, and reword the bus comment about the three aggregated events: they reply with the final output as is, unlike Stop and MessageDisplay. --- packages/cli/src/config/settings.test.ts | 42 +++++++++++++++++++ .../cli/src/config/settingsSchema.test.ts | 18 ++++++++ packages/core/src/config/config.ts | 6 ++- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index f9b2b09cd65..e528a3bc629 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -1177,6 +1177,48 @@ describe('Settings Loading and Merging', () => { expect(settings.merged.advanced?.excludedEnvVars).toHaveLength(2); }); + it('should concatenate hook definitions from user and workspace scopes', () => { + (mockFsExistsSync as Mock).mockReturnValue(true); + const hookRunning = (command: string) => [ + { hooks: [{ type: 'command', command }] }, + ]; + const userSettings = { + hooks: { + PostCompact: hookRunning('user-post-compact'), + TodoCreated: hookRunning('user-todo-created'), + }, + }; + const workspaceSettings = { + hooks: { + PostCompact: hookRunning('workspace-post-compact'), + TodoCreated: hookRunning('workspace-todo-created'), + }, + }; + + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) return JSON.stringify(userSettings); + if (p === MOCK_WORKSPACE_SETTINGS_PATH) + return JSON.stringify(workspaceSettings); + return '{}'; + }, + ); + + const settings = loadSettings(MOCK_WORKSPACE_DIR); + + // Both scopes run: a workspace definition does not replace the user's. + expect(settings.merged.hooks).toMatchObject({ + PostCompact: [ + { hooks: [{ command: 'user-post-compact' }] }, + { hooks: [{ command: 'workspace-post-compact' }] }, + ], + TodoCreated: [ + { hooks: [{ command: 'user-todo-created' }] }, + { hooks: [{ command: 'workspace-todo-created' }] }, + ], + }); + }); + it('should UNION-merge slashCommands.disabled across user and workspace scopes', () => { (mockFsExistsSync as Mock).mockReturnValue(true); const userSettings = { diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 0bdd16a3fc8..d87989b7c1a 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -52,6 +52,24 @@ describe('SettingsSchema', () => { ).toEqual([...Object.values(HookEventName)].sort()); }); + it('should concatenate every hook event across settings scopes', () => { + const hookProperties = getSettingsSchema().hooks.properties ?? {}; + const eventNames = Object.values(HookEventName); + + expect( + Object.fromEntries( + eventNames.map((eventName) => [ + eventName, + hookProperties[eventName]?.mergeStrategy, + ]), + ), + ).toEqual( + Object.fromEntries( + eventNames.map((eventName) => [eventName, MergeStrategy.CONCAT]), + ), + ); + }); + it('should contain all expected top-level settings', () => { const expectedSettings: Array = [ 'mcpServers', diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 3d685b09df6..aceb8161047 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -3622,8 +3622,10 @@ export class Config { signal, ); break; - // These three return the aggregated result; the bus carries its - // final output like every other event. + // These three return the aggregated result. The bus replies with + // its final output as is, which is what direct callers read + // (todoWrite checks `finalOutput.decision`); Stop and + // MessageDisplay instead wrap theirs with createHookOutput. case 'StopFailure': result = ( await hookSystem.fireStopFailureEvent( From 58f72d2dea96c8ed09ce2a3cbf70947e9147cef8 Mon Sep 17 00:00:00 2001 From: qqqys Date: Sat, 12 Sep 2026 01:23:10 +0800 Subject: [PATCH 3/3] test(core): pin every hook event and the forwarded signal on the hook bus The schema already fails when a hook event lacks an entry; the bus switch had no such guard, so a new event could fall through to the unknown-event default with the suite green. Iterate HookEventName and require each event to reach a hook system method. The direct-call bridge tests now send a real abort signal and assert the same signal is forwarded, cover the no-hook-configured path that replies with no output, use a declared AgentType in the SessionStart example, and give the Todo events two distinct outputs so a first-output projection is caught. StopFailure gets its own case with the aggregate shape the aggregator really returns, and the comment on its arm now says it is fire-and-forget and always replies with no output. --- packages/core/src/config/config.test.ts | 188 ++++++++++++++++++++---- packages/core/src/config/config.ts | 13 +- 2 files changed, 172 insertions(+), 29 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 722ffc7688c..8e7b002b7c6 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -89,6 +89,7 @@ import { RipgrepFallbackEvent } from '../telemetry/types.js'; import { ToolRegistry } from '../tools/tool-registry.js'; import { ToolNames } from '../tools/tool-names.js'; import { fireNotificationHook } from '../core/toolHookTriggers.js'; +import { AgentType, HookEventName } from '../hooks/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { MessageBusType, @@ -13724,12 +13725,69 @@ describe('Model Switching and Config Updates', () => { }); }); + describe('every hook event through the hook execution bridge', () => { + // The schema side has a drift guard derived from HookEventName; this is + // the bus side. `eventName` is an open string on the wire, so the compiler + // cannot catch a missing case, and `default:` replies with the same empty + // success a real no-op produces. + it.each(Object.values(HookEventName))( + 'routes %s to a hook system method instead of the unknown-event default', + async (eventName) => { + const config = new Config({ ...baseParams }); + await config.initialize(); + const called: string[] = []; + // Every fire method resolves an empty aggregate, which each arm accepts. + const hookSystem = new Proxy( + {}, + { + get: (_target, prop) => { + if (typeof prop !== 'string' || prop === 'then') { + return undefined; + } + return vi.fn(async () => { + called.push(prop); + return { + success: true, + allOutputs: [], + errors: [], + totalDuration: 0, + finalOutput: undefined, + }; + }); + }, + }, + ); + // @ts-expect-error - accessing private for testing + config['hookSystem'] = hookSystem; + const warn = vi.spyOn(config.getDebugLogger(), 'warn'); + + const response = await config + .getMessageBus()! + .request( + { + type: MessageBusType.HOOK_EXECUTION_REQUEST, + eventName, + input: {}, + }, + MessageBusType.HOOK_EXECUTION_RESPONSE, + ); + + expect(warn).not.toHaveBeenCalledWith( + expect.stringContaining('Unknown hook event'), + ); + expect(called.some((method) => method.startsWith('fire'))).toBe(true); + expect(response.success).toBe(true); + }, + ); + }); + describe('direct-call hook events through the hook execution bridge', () => { const dispatch = async ( method: string, fire: ReturnType, eventName: string, input: Record, + signal: AbortSignal, ) => { const config = new Config({ ...baseParams }); await config.initialize(); @@ -13737,13 +13795,20 @@ describe('Model Switching and Config Updates', () => { config['hookSystem'] = { [method]: fire }; return config .getMessageBus()! - .request< - HookExecutionRequest, - HookExecutionResponse - >({ type: MessageBusType.HOOK_EXECUTION_REQUEST, eventName, input }, MessageBusType.HOOK_EXECUTION_RESPONSE); + .request( + { + type: MessageBusType.HOOK_EXECUTION_REQUEST, + eventName, + input, + signal, + }, + MessageBusType.HOOK_EXECUTION_RESPONSE, + ); }; - it.each([ + // Events whose fire method returns the hook output itself, or undefined + // when no hook is configured. + const directOutputRows = [ { eventName: 'SessionStart', method: 'fireSessionStartEvent', @@ -13751,9 +13816,9 @@ describe('Model Switching and Config Updates', () => { source: 'resume', model: 'qwen-max', permission_mode: 'plan', - agent_type: 'general-purpose', + agent_type: 'Custom', }, - args: ['resume', 'qwen-max', 'plan', 'general-purpose'], + args: ['resume', 'qwen-max', 'plan', 'Custom'], }, { eventName: 'SessionEnd', @@ -13799,31 +13864,61 @@ describe('Model Switching and Config Updates', () => { }, ], }, - ])( + ]; + + it('uses a declared AgentType in the SessionStart wire example', () => { + // The wire carries a raw string and nothing downstream validates it, so + // this row is the example an out-of-process producer copies. + const row = directOutputRows.find( + ({ eventName }) => eventName === 'SessionStart', + )!; + expect(Object.values(AgentType)).toContain(row.input['agent_type']); + }); + + it.each(directOutputRows)( 'forwards $eventName to $method', async ({ eventName, method, input, args }) => { const output = { systemMessage: `${eventName} ran` }; const fire = vi.fn().mockResolvedValue(output); + const controller = new AbortController(); + + const response = await dispatch( + method, + fire, + eventName, + input, + controller.signal, + ); - const response = await dispatch(method, fire, eventName, input); - - expect(fire).toHaveBeenCalledWith(...args, undefined); + expect(fire).toHaveBeenCalledWith(...args, controller.signal); expect(response.success).toBe(true); expect(response.output).toEqual(output); }, ); - it.each([ - { - eventName: 'StopFailure', - method: 'fireStopFailureEvent', - input: { - error: 'rate_limit', - error_details: '429 Too Many Requests', - last_assistant_message: 'partial', - }, - args: ['rate_limit', '429 Too Many Requests', 'partial'], + it.each(directOutputRows)( + 'replies with no output when no $eventName hook is configured', + async ({ eventName, method, input, args }) => { + const fire = vi.fn().mockResolvedValue(undefined); + const controller = new AbortController(); + + const response = await dispatch( + method, + fire, + eventName, + input, + controller.signal, + ); + + // The call assertion also tells this arm apart from `default:`, which + // publishes the same empty success reply without calling anything. + expect(fire).toHaveBeenCalledWith(...args, controller.signal); + expect(response.success).toBe(true); + expect(response.output).toBeUndefined(); }, + ); + + it.each([ { eventName: 'TodoCreated', method: 'fireTodoCreatedEvent', @@ -13851,22 +13946,65 @@ describe('Model Switching and Config Updates', () => { ])( 'forwards $eventName to $method and returns its final output', async ({ eventName, method, input, args }) => { + // Two distinct outputs whose merge differs from the first, so + // replying with one hook's output instead of the merged result fails. const finalOutput = { decision: 'block', reason: 'not yet' }; const fire = vi.fn().mockResolvedValue({ success: true, - allOutputs: [finalOutput], + allOutputs: [{ decision: 'allow' }, finalOutput], errors: [], totalDuration: 1, finalOutput, }); + const controller = new AbortController(); + + const response = await dispatch( + method, + fire, + eventName, + input, + controller.signal, + ); - const response = await dispatch(method, fire, eventName, input); - - expect(fire).toHaveBeenCalledWith(...args, undefined); + expect(fire).toHaveBeenCalledWith(...args, controller.signal); expect(response.success).toBe(true); expect(response.output).toEqual(finalOutput); }, ); + + it('awaits StopFailure hooks but replies with no output', async () => { + // The shape HookAggregator returns for StopFailure: fire-and-forget, + // outputs and errors dropped, no final output. + const fire = vi.fn().mockResolvedValue({ + success: true, + allOutputs: [], + errors: [], + totalDuration: 3, + finalOutput: undefined, + }); + const controller = new AbortController(); + + const response = await dispatch( + 'fireStopFailureEvent', + fire, + 'StopFailure', + { + error: 'rate_limit', + error_details: '429 Too Many Requests', + last_assistant_message: 'partial', + }, + controller.signal, + ); + + expect(fire).toHaveBeenCalledWith( + 'rate_limit', + '429 Too Many Requests', + 'partial', + controller.signal, + ); + expect(response.success).toBe(true); + expect(response.output).toBeUndefined(); + }); }); describe('Stop dispatch through the hook execution bridge', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 6ec64106f3a..89868fbf0de 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -3640,10 +3640,15 @@ export class Config { signal, ); break; - // These three return the aggregated result. The bus replies with - // its final output as is, which is what direct callers read - // (todoWrite checks `finalOutput.decision`); Stop and - // MessageDisplay instead wrap theirs with createHookOutput. + // These three return the aggregated result, and the bus replies + // with its final output as is. For TodoCreated and TodoCompleted + // that is what direct callers read (todoWrite checks + // `finalOutput.decision`). StopFailure is fire-and-forget: the + // aggregator hard-codes its `finalOutput` to undefined and every + // direct caller detaches without reading the result, so its arm + // always replies with no output and awaits only so the hooks run. + // Stop and MessageDisplay instead wrap theirs with + // createHookOutput. case 'StopFailure': result = ( await hookSystem.fireStopFailureEvent(