diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts index 8025b3ea67598..411f81483df3a 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts @@ -47,6 +47,55 @@ 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. + * Uses JSON.stringify for unambiguous encoding (avoids collisions from + * separator characters in env keys/values). + */ +function computeHookSignature(hook: IHookCommand): string { + 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, + }); +} + +/** + * 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: readonly IHookCommand[]): readonly 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..c23e1484f597a 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'); + }); + }); });