fix(plugins): use mergeHooksSettings in marketplace supplement path (#1055) (#1167)

`finishLoadingPluginFromPath` supplemented `plugin.hooksConfig` with the
marketplace entry's hooks via object spread:

    plugin.hooksConfig = {
      ...(plugin.hooksConfig || {}),
      ...(entry.hooks as HooksSettings),
    }

`HooksSettings` values are matcher arrays keyed by event name. Object
spread replaced the entire per-event array from `plugin.json` with the
marketplace entry's array, silently dropping any matchers the manifest
already registered for the same event (e.g. both contributed
`PreToolUse` matchers — only the marketplace ones survived).

`mergeHooksSettings` already exists in this file and concatenates
per-event arrays correctly; it is the helper used in
`createPluginFromPath` for the analogous merge. Use it in the
marketplace supplement path too, and export it so the concat-not-replace
contract is locked in by a unit test.

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,

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,
+      )
     }
   }