diff --git a/src/utils/plugins/pluginLoader.test.ts b/src/utils/plugins/pluginLoader.test.ts index 99818752d..f122b0cd0 100644 --- a/src/utils/plugins/pluginLoader.test.ts +++ b/src/utils/plugins/pluginLoader.test.ts @@ -12,6 +12,7 @@ import type { LoadedPlugin } from '../../types/plugin.js' import { clearPluginCache, createPluginFromPath, + mergeHooksSettings, mergePluginSources, resolveExistingPluginComponentPath, resolvePluginComponentPath, @@ -99,6 +100,55 @@ describe('mergePluginSources', () => { }) }) +describe('mergeHooksSettings', () => { + // Regression for #1055: the marketplace-supplement path previously used + // object spread, which replaced the entire per-event matcher array from + // plugin.json with the marketplace entry's array. mergeHooksSettings is + // the helper that must be used at that call site; this test locks in + // the concat-not-replace contract the call site depends on. + test('concatenates per-event matcher arrays instead of replacing them', () => { + const base = { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'A' }] }], + } as never + const additional = { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'B' }] }], + } as never + + const merged = mergeHooksSettings(base, additional) as never as { + PreToolUse: Array<{ hooks: Array<{ command: string }> }> + } + + expect(merged.PreToolUse).toHaveLength(2) + expect(merged.PreToolUse[0].hooks[0].command).toBe('A') + expect(merged.PreToolUse[1].hooks[0].command).toBe('B') + }) + + test('keeps disjoint events from both inputs', () => { + const base = { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'pre' }] }], + } as never + const additional = { + PostToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'post' }] }], + } as never + + const merged = mergeHooksSettings(base, additional) as never as { + PreToolUse: unknown[] + PostToolUse: unknown[] + } + + expect(merged.PreToolUse).toHaveLength(1) + expect(merged.PostToolUse).toHaveLength(1) + }) + + test('returns additional unchanged when base is undefined', () => { + const additional = { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'X' }] }], + } as never + + expect(mergeHooksSettings(undefined, additional)).toBe(additional) + }) +}) + async function createDirectoryLink( target: string, linkPath: string, diff --git a/src/utils/plugins/pluginLoader.ts b/src/utils/plugins/pluginLoader.ts index 4324b2603..f6a07768c 100644 --- a/src/utils/plugins/pluginLoader.ts +++ b/src/utils/plugins/pluginLoader.ts @@ -2022,9 +2022,12 @@ async function loadPluginSettings( } /** - * Merge two HooksSettings objects + * Merge two HooksSettings objects. Concatenates matcher arrays for events + * that appear in both `base` and `additional`. Exported so callers (and + * tests) can verify the supplement path keeps both sets of matchers + * rather than dropping the base ones via object spread. */ -function mergeHooksSettings( +export function mergeHooksSettings( base: HooksSettings | undefined, additional: HooksSettings, ): HooksSettings { @@ -3141,12 +3144,15 @@ async function finishLoadingPluginFromPath( } } - // Supplement hooks from marketplace entry + // Supplement hooks from marketplace entry. Object spread would replace + // each event's matcher array wholesale, silently dropping matchers that + // plugin.json already registered for the same event. mergeHooksSettings + // concatenates per-event arrays the way the rest of this file does. if (entry.hooks) { - plugin.hooksConfig = { - ...(plugin.hooksConfig || {}), - ...(entry.hooks as HooksSettings), - } + plugin.hooksConfig = mergeHooksSettings( + plugin.hooksConfig, + entry.hooks as HooksSettings, + ) } }