Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/vs/workbench/contrib/chat/common/promptSyntax/hookSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to also actually get the list of files that were deduped for debug events. I can take a look at that though because that flow needs some caching changes. Thanks!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the state here, should I close this?

}
collectedHooks.set(hookType, [...deduped]);
}

// Build the result
const result: ChatRequestHooks = Object.fromEntries(collectedHooks) as ChatRequestHooks;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
});
});
});
Loading