fix(plugins): use mergeHooksSettings() for marketplace hook supplement

src/utils/plugins/pluginLoader.test.ts

@@ -1,13 +1,18 @@
-import { mkdtemp, mkdir, rm, symlink, writeFile } from 'fs/promises'
-import { tmpdir } from 'os'
-import { join, resolve } from 'path'
-import { afterEach, describe, expect, test } from 'bun:test'
+import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
+
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
 
 import { setInlinePlugins } from '../../bootstrap/state.js'
 import type { LoadedPlugin } from '../../types/plugin.js'
+import type { HooksSettings } from '../settings/types.js'
+import type { PluginMarketplaceEntry } from './schemas.js'
 import {
   clearPluginCache,
   createPluginFromPath,
+  finishLoadingPluginFromPath,
+  mergeHooksSettings,
   mergePluginSources,
   resolveExistingPluginComponentPath,
   resolvePluginComponentPath,
@@ -36,6 +41,60 @@ function marketplacePlugin(
   }
 }
 
+// ---------------------------------------------------------------------------
+// mergeHooksSettings — validates the marketplace supplement loader path
+// (finishLoadingPluginFromPath, marketplace hook supplement block). Before
+// this fix, the supplement used object spread ({...plugin.hooksConfig,
+// ...entry.hooks}) which silently overwrote same-event matcher arrays from
+// plugin.json with the marketplace arrays.
+// ---------------------------------------------------------------------------
+describe('mergeHooksSettings', () => {
+  test('appends marketplace matchers to plugin.json matchers for the same event', () => {
+    // Simulates plugin.json defining a PostToolUse hook
+    const pluginJsonHooks: HooksSettings = {
+      PostToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'plugin-json-hook' }] }],
+    }
+    // Simulates marketplace entry supplementing the same event
+    const marketplaceHooks: HooksSettings = {
+      PostToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'marketplace-hook' }] }],
+    }
+
+    const merged = mergeHooksSettings(pluginJsonHooks, marketplaceHooks)
+
+    // Both matchers must be present — marketplace must not overwrite plugin.json
+    expect(merged.PostToolUse).toHaveLength(2)
+    const commands = (merged.PostToolUse as Array<{ hooks: Array<{ command?: string }> }>).map(
+      m => m.hooks[0]?.command,
+    )
+    expect(commands).toContain('plugin-json-hook')
+    expect(commands).toContain('marketplace-hook')
+  })
+
+  test('adds marketplace event when plugin.json has no hook for it', () => {
+    const pluginJsonHooks: HooksSettings = {
+      PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'pre-hook' }] }],
+    }
+    const marketplaceHooks: HooksSettings = {
+      PostToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'post-hook' }] }],
+    }
+
+    const merged = mergeHooksSettings(pluginJsonHooks, marketplaceHooks)
+
+    expect(merged.PreToolUse).toHaveLength(1)
+    expect(merged.PostToolUse).toHaveLength(1)
+  })
+
+  test('returns marketplace hooks when plugin.json has no hooks at all', () => {
+    const marketplaceHooks: HooksSettings = {
+      PostToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'mkt-hook' }] }],
+    }
+
+    const merged = mergeHooksSettings(undefined, marketplaceHooks)
+
+    expect(merged).toEqual(marketplaceHooks)
+  })
+})
+
 describe('mergePluginSources', () => {
   test('keeps the enabled copy when duplicate marketplace plugins disagree on enabled state', () => {
     const enabledOfficial = marketplacePlugin(
@@ -445,3 +504,66 @@ describe('resolvePluginComponentPath', () => {
     }
   })
 })
+
+describe('finishLoadingPluginFromPath — marketplace hook supplement', () => {
+  let tmpDir: string
+
+  beforeEach(async () => {
+    tmpDir = await mkdtemp(join(tmpdir(), 'openclaude-plugin-test-'))
+  })
+
+  afterEach(async () => {
+    await rm(tmpDir, { recursive: true, force: true })
+  })
+
+  test('concatenates marketplace entry hooks with plugin.json hooks for the same event', async () => {
+    // Arrange: write a minimal plugin on disk with a PreToolUse hook
+    await mkdir(join(tmpDir, '.claude-plugin'), { recursive: true })
+    await writeFile(
+      join(tmpDir, '.claude-plugin', 'plugin.json'),
+      JSON.stringify({ name: 'test-plugin', version: '1.0.0' }),
+    )
+    await mkdir(join(tmpDir, 'hooks'), { recursive: true })
+    await writeFile(
+      join(tmpDir, 'hooks', 'hooks.json'),
+      JSON.stringify({
+        hooks: {
+          PreToolUse: [
+            { matcher: 'Bash', hooks: [{ type: 'command', command: 'echo from-plugin-json' }] },
+          ],
+        },
+      }),
+    )
+
+    // Marketplace entry supplements PreToolUse with an additional matcher
+    const entry = {
+      name: 'test-plugin',
+      source: 'test-source',
+      strict: true,
+      hooks: {
+        PreToolUse: [
+          { matcher: 'Write', hooks: [{ type: 'command', command: 'echo from-marketplace' }] },
+        ],
+      },
+    } as unknown as PluginMarketplaceEntry
+
+    const errors: Parameters<typeof finishLoadingPluginFromPath>[3] = []
+
+    // Act: run the actual loader path
+    const plugin = await finishLoadingPluginFromPath(
+      entry,
+      'test-plugin@marketplace',
+      true,
+      errors,
+      tmpDir,
+    )
+
+    // Assert: both matchers are present in order — plugin.json first, marketplace second
+    expect(plugin).not.toBeNull()
+    const preToolUse = plugin!.hooksConfig?.PreToolUse as Array<{ matcher: string }>
+    expect(preToolUse).toHaveLength(2)
+    expect(preToolUse[0]!.matcher).toBe('Bash')
+    expect(preToolUse[1]!.matcher).toBe('Write')
+    expect(errors).toHaveLength(0)
+  })
+})

src/utils/plugins/pluginLoader.ts

@@ -2022,9 +2022,12 @@ async function loadPluginSettings(
 }
 
 /**
- * Merge two HooksSettings objects
+ * Merge two HooksSettings objects, appending matchers for shared events.
+ * Exported for targeted unit tests of the marketplace supplement loader path
+ * (see finishLoadingPluginFromPath, marketplace hook supplement block).
+ * Callers outside this module should not depend on this export.
  */
-function mergeHooksSettings(
+export function mergeHooksSettings(
   base: HooksSettings | undefined,
   additional: HooksSettings,
 ): HooksSettings {
@@ -2590,7 +2593,7 @@ async function loadPluginFromMarketplaceEntry(
  * entry supplementation — is identical. Extracted so the cache-only path
  * doesn't duplicate ~500 lines.
  */
-async function finishLoadingPluginFromPath(
+export async function finishLoadingPluginFromPath(
   entry: PluginMarketplaceEntry,
   pluginId: string,
   enabled: boolean,
@@ -3141,12 +3144,16 @@ async function finishLoadingPluginFromPath(
       }
     }
 
-    // Supplement hooks from marketplace entry
+    // Supplement hooks from marketplace entry.
+    // CORRECTNESS: Use mergeHooksSettings() instead of object spread so that
+    // per-event matcher arrays from plugin.json and the marketplace entry are
+    // concatenated rather than the marketplace entry silently overwriting the
+    // plugin.json matchers for the same event.
     if (entry.hooks) {
-      plugin.hooksConfig = {
-        ...(plugin.hooksConfig || {}),
-        ...(entry.hooks as HooksSettings),
-      }
+      plugin.hooksConfig = mergeHooksSettings(
+        plugin.hooksConfig,
+        entry.hooks as HooksSettings,
+      )
     }
   }