From 100e947d1ed5cc6854d93e6de5071fa4f6ad915a Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Sun, 22 Mar 2026 18:14:25 -0500 Subject: [PATCH 1/3] Deduplicate hooks from multiple discovery paths When the same hook is defined in both .github/hooks/ and .claude/settings.json, computeHooks() collects both and fires them twice per tool call. This adds a deduplication step after all hooks are collected but before converting to immutable ChatRequestHooks. Two hooks are considered identical if all their command fields (command, windows, linux, osx), cwd, env, and timeout match. First occurrence is kept, preserving the path priority order from DEFAULT_HOOK_FILE_PATHS. Fixes #303926 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../chat/common/promptSyntax/hookSchema.ts | 54 ++++++++ .../service/promptsServiceImpl.ts | 12 +- .../common/promptSyntax/hookSchema.test.ts | 117 +++++++++++++++++- 3 files changed, 181 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts index 8025b3ea67598..248604d2c6714 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts @@ -47,6 +47,60 @@ export type ChatRequestHooks = { readonly [K in HookType]?: readonly IHookCommand[]; }; +/** + * Computes a signature string for a hook command that captures all fields + * which determine whether two hooks would execute identically. + * Used for deduplication when the same hook is defined in multiple discovery paths. + */ +function computeHookSignature(hook: IHookCommand): string { + const parts = [ + hook.command ?? '', + hook.windows ?? '', + hook.linux ?? '', + hook.osx ?? '', + hook.cwd?.toString() ?? '', + hook.timeout !== undefined ? String(hook.timeout) : '', + ]; + + if (hook.env) { + const sortedEnv = Object.entries(hook.env) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([k, v]) => `${k}=${v}`) + .join('|'); + parts.push(sortedEnv); + } else { + parts.push(''); + } + + return parts.join('\0'); +} + +/** + * Removes duplicate hook commands from an array. + * Two hooks are considered identical if all their command fields (command, windows, + * linux, osx), cwd, env, and timeout match. This handles the case where the same + * hook is defined in multiple discovery paths (e.g., `.github/hooks/` and + * `.claude/settings.json`). First occurrence is kept, preserving path priority order. + */ +export function deduplicateHooks(hooks: IHookCommand[]): IHookCommand[] { + if (hooks.length <= 1) { + return hooks; + } + + const seen = new Set(); + const result: IHookCommand[] = []; + + for (const hook of hooks) { + const signature = computeHookSignature(hook); + if (!seen.has(signature)) { + seen.add(signature); + result.push(hook); + } + } + + return result; +} + /** * Merges two sets of hooks by concatenating the command arrays for each hook type. * Additional hooks are appended after the base hooks. diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index 3a216261dedf5..bdf5e1f05a159 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -39,7 +39,7 @@ import { PromptFileParser, ParsedPromptFile, PromptHeaderAttributes } from '../p import { IAgentInstructions, type IAgentSource, IChatPromptSlashCommand, IConfiguredHooksInfo, ICustomAgent, IExtensionPromptPath, ILocalPromptPath, IPluginPromptPath, IPromptPath, IPromptsService, IAgentSkill, IUserPromptPath, PromptsStorage, ExtensionAgentSourceType, CUSTOM_AGENT_PROVIDER_ACTIVATION_EVENT, INSTRUCTIONS_PROVIDER_ACTIVATION_EVENT, IPromptFileContext, IPromptFileResource, PROMPT_FILE_PROVIDER_ACTIVATION_EVENT, SKILL_PROVIDER_ACTIVATION_EVENT, IPromptDiscoveryInfo, IPromptFileDiscoveryResult, IPromptSourceFolderResult, ICustomAgentVisibility, IResolvedAgentFile, AgentFileType, Logger, IPromptDiscoveryLogEntry } from './promptsService.js'; import { Delayer } from '../../../../../../base/common/async.js'; import { Schemas } from '../../../../../../base/common/network.js'; -import { ChatRequestHooks, IHookCommand, parseSubagentHooksFromYaml } from '../hookSchema.js'; +import { ChatRequestHooks, IHookCommand, deduplicateHooks, parseSubagentHooksFromYaml } from '../hookSchema.js'; import { HookType } from '../hookTypes.js'; import { HookSourceFormat, getHookSourceFormat, parseHooksFromFile } from '../hookCompatibility.js'; import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; @@ -1367,6 +1367,16 @@ export class PromptsService extends Disposable implements IPromptsService { return undefined; } + // Deduplicate hooks that appear in multiple discovery paths + // (e.g., same hook in both .github/hooks/ and .claude/settings.json) + for (const [hookType, commands] of collectedHooks) { + const deduped = deduplicateHooks(commands); + if (deduped.length < commands.length) { + this.logger.trace(`[PromptsService] Deduplicated ${hookType} hooks: ${commands.length} → ${deduped.length}`); + } + collectedHooks.set(hookType, deduped); + } + // Build the result const result: ChatRequestHooks = Object.fromEntries(collectedHooks) as ChatRequestHooks; diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookSchema.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookSchema.test.ts index ee30e3f60dc08..c85c15c48f092 100644 --- a/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookSchema.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookSchema.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; -import { resolveHookCommand, resolveEffectiveCommand, formatHookCommandLabel, IHookCommand, parseSubagentHooksFromYaml } from '../../../common/promptSyntax/hookSchema.js'; +import { resolveHookCommand, resolveEffectiveCommand, formatHookCommandLabel, IHookCommand, deduplicateHooks, parseSubagentHooksFromYaml } from '../../../common/promptSyntax/hookSchema.js'; import { URI } from '../../../../../../base/common/uri.js'; import { OperatingSystem } from '../../../../../../base/common/platform.js'; import { HookType } from '../../../common/promptSyntax/hookTypes.js'; @@ -645,4 +645,119 @@ suite('HookSchema', () => { assert.strictEqual(result[HookType.PreToolUse], undefined); }); }); + + suite('deduplicateHooks', () => { + test('returns empty array unchanged', () => { + const result = deduplicateHooks([]); + assert.strictEqual(result.length, 0); + }); + + test('returns single hook unchanged', () => { + const hooks: IHookCommand[] = [{ type: 'command', command: 'echo test' }]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 1); + assert.deepStrictEqual(result[0], hooks[0]); + }); + + test('removes identical duplicate commands', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'echo hello' }, + { type: 'command', command: 'echo hello' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 1); + assert.deepStrictEqual(result[0], hooks[0]); + }); + + test('preserves order of first occurrence', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'first' }, + { type: 'command', command: 'second' }, + { type: 'command', command: 'first' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + assert.strictEqual(result[0].command, 'first'); + assert.strictEqual(result[1].command, 'second'); + }); + + test('keeps hooks with different commands', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'echo a' }, + { type: 'command', command: 'echo b' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + }); + + test('keeps hooks with different cwd', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'npm test', cwd: URI.file('/workspace') }, + { type: 'command', command: 'npm test', cwd: URI.file('/workspace/src') }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + }); + + test('keeps hooks with different timeout', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'validate', timeout: 30 }, + { type: 'command', command: 'validate', timeout: 60 }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + }); + + test('keeps hooks with different env', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'npm test', env: { DEBUG: '1' } }, + { type: 'command', command: 'npm test' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + }); + + test('deduplicates hooks with identical env (order independent)', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'test', env: { A: '1', B: '2' } }, + { type: 'command', command: 'test', env: { B: '2', A: '1' } }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 1); + }); + + test('keeps hooks with different platform overrides', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'validate' }, + { type: 'command', command: 'validate', windows: 'validate.bat' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 2); + }); + + test('deduplicates cross-path hooks with identical fields', () => { + // Simulates same hook defined in .github/hooks/ and .claude/settings.json + const hooks: IHookCommand[] = [ + { type: 'command', command: 'echo "AI disclosure reminder"', timeout: 5 }, + { type: 'command', command: 'echo "AI disclosure reminder"', timeout: 5 }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 1); + }); + + test('deduplicates multiple duplicates across many entries', () => { + const hooks: IHookCommand[] = [ + { type: 'command', command: 'hook-a' }, + { type: 'command', command: 'hook-b' }, + { type: 'command', command: 'hook-a' }, + { type: 'command', command: 'hook-c' }, + { type: 'command', command: 'hook-b' }, + ]; + const result = deduplicateHooks(hooks); + assert.strictEqual(result.length, 3); + assert.strictEqual(result[0].command, 'hook-a'); + assert.strictEqual(result[1].command, 'hook-b'); + assert.strictEqual(result[2].command, 'hook-c'); + }); + }); }); From d6f816d61518abc71a4337415992be5de79a8e6e Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Sun, 22 Mar 2026 18:28:02 -0500 Subject: [PATCH 2/3] Address review: JSON.stringify for signature, readonly parameter - Use JSON.stringify instead of NUL-joined parts for hook signature encoding to avoid ambiguity from separator chars in env keys/values - Accept readonly IHookCommand[] in deduplicateHooks() to match ChatRequestHooks readonly arrays Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../chat/common/promptSyntax/hookSchema.ts | 37 ++++++++----------- .../service/promptsServiceImpl.ts | 2 +- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts index 248604d2c6714..411f81483df3a 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts @@ -51,28 +51,23 @@ export type ChatRequestHooks = { * Computes a signature string for a hook command that captures all fields * which determine whether two hooks would execute identically. * Used for deduplication when the same hook is defined in multiple discovery paths. + * Uses JSON.stringify for unambiguous encoding (avoids collisions from + * separator characters in env keys/values). */ function computeHookSignature(hook: IHookCommand): string { - const parts = [ - hook.command ?? '', - hook.windows ?? '', - hook.linux ?? '', - hook.osx ?? '', - hook.cwd?.toString() ?? '', - hook.timeout !== undefined ? String(hook.timeout) : '', - ]; - - if (hook.env) { - const sortedEnv = Object.entries(hook.env) - .sort(([a], [b]) => a.localeCompare(b)) - .map(([k, v]) => `${k}=${v}`) - .join('|'); - parts.push(sortedEnv); - } else { - parts.push(''); - } - - return parts.join('\0'); + const envEntries = hook.env + ? Object.entries(hook.env).sort(([a], [b]) => a.localeCompare(b)) + : []; + + return JSON.stringify({ + command: hook.command ?? '', + windows: hook.windows ?? '', + linux: hook.linux ?? '', + osx: hook.osx ?? '', + cwd: hook.cwd?.toString() ?? '', + timeout: hook.timeout ?? null, + env: envEntries.length > 0 ? envEntries : null, + }); } /** @@ -82,7 +77,7 @@ function computeHookSignature(hook: IHookCommand): string { * hook is defined in multiple discovery paths (e.g., `.github/hooks/` and * `.claude/settings.json`). First occurrence is kept, preserving path priority order. */ -export function deduplicateHooks(hooks: IHookCommand[]): IHookCommand[] { +export function deduplicateHooks(hooks: readonly IHookCommand[]): readonly IHookCommand[] { if (hooks.length <= 1) { return hooks; } diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index bdf5e1f05a159..77d708e9b7e2c 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -1374,7 +1374,7 @@ export class PromptsService extends Disposable implements IPromptsService { if (deduped.length < commands.length) { this.logger.trace(`[PromptsService] Deduplicated ${hookType} hooks: ${commands.length} → ${deduped.length}`); } - collectedHooks.set(hookType, deduped); + collectedHooks.set(hookType, deduped as IHookCommand[]); } // Build the result From 24dc99665506992d97363f37e7cd75193a36720f Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Sun, 22 Mar 2026 18:46:47 -0500 Subject: [PATCH 3/3] Replace as-cast with spread to preserve type safety Use [...deduped] instead of 'as IHookCommand[]' cast to maintain mutable array for the collectedHooks Map without weakening the readonly return type of deduplicateHooks(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../chat/common/promptSyntax/service/promptsServiceImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index 77d708e9b7e2c..c23e1484f597a 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -1374,7 +1374,7 @@ export class PromptsService extends Disposable implements IPromptsService { if (deduped.length < commands.length) { this.logger.trace(`[PromptsService] Deduplicated ${hookType} hooks: ${commands.length} → ${deduped.length}`); } - collectedHooks.set(hookType, deduped as IHookCommand[]); + collectedHooks.set(hookType, [...deduped]); } // Build the result