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

[0xghost42]

Fixes #1055.

Problem

In finishLoadingPluginFromPath, the marketplace supplement path was building plugin.hooksConfig with an object spread:

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

`HooksSettings` values are matcher arrays keyed by event name (`PreToolUse`, `PostToolUse`, ...). Object spread replaces the entire per-event array from `plugin.json` with the marketplace entry's array, silently dropping any matchers the manifest already registered for that event.

`mergeHooksSettings` already exists in this file (used by `createPluginFromPath` for the analogous merge) and concatenates per-event matcher arrays correctly. It was just never wired into the marketplace supplement path.

Fix

• Switch the supplement path to `mergeHooksSettings(plugin.hooksConfig, entry.hooks as HooksSettings)`.
• Export `mergeHooksSettings` so the concat-not-replace contract that the call site now depends on can be locked in by a unit test.

Tests

Added three unit tests in `pluginLoader.test.ts`:

• Same-event matchers from `base` and `additional` end up concatenated (regression case for fix(plugins): marketplace hook supplement drops plugin.json matchers (missing mergeHooksSettings) #1055).
• Disjoint events from both inputs are preserved.
• `additional` is returned unchanged when `base` is `undefined`.

```
bun test src/utils/plugins/pluginLoader.test.ts
16 pass
0 fail
```

[gnanam1990]

Local: bun test src/utils/plugins/pluginLoader.test.ts → 16/0 pass.

Clean, focused fix. Object-spread on HooksSettings (matcher arrays keyed by event name) was dropping the manifest's matchers whenever the marketplace entry had a matcher for the same event — mergeHooksSettings is exactly the right helper, and it's already the established pattern at lines 1888 and 1925 in the same file. Exporting it for the test is fine.

No red flags. LGTM.

[jatmn]

No issues here, LGTM.

[Vasanthdev2004]

Blockers

None.

Non-Blocking

None.

Looks Good

• Fixes object-spread bug that was dropping manifest matchers
• Uses existing mergeHooksSettings helper (correct pattern)
• 3 unit tests covering the regression case
• 16 tests passing
• Clean, focused change (63 additions, 7 deletions)

---

Verdict: Approve — clean plugin hook merge fix.

[Vasanthdev2004]

Clean plugin hook merge fix. No blockers.

[techbrewboss]

Review summary

No blocking or actionable issues found.

This is a focused, useful fix for marketplace plugin hook loading. The change reuses the existing hook merge helper instead of object-spreading HooksSettings, preserving both manifest and marketplace matchers for the same event. The added tests cover the regression and adjacent merge behavior.

Findings

No findings.

Value

The PR adds clear project value: it fixes the hook matcher loss described in #1055 without broadening scope or changing unrelated plugin loading behavior.

Maliciousness / Risk

No suspicious behavior found. The PR adds no dependencies, network calls, shell execution, install hooks, credential access, or permission broadening. bun run security:pr-scan reported: PR intent scan: no suspicious additions found.

Validation

• bun test src/utils/plugins/pluginLoader.test.ts passed: 16 pass, 0 fail.
• git diff --check main...HEAD passed.
• bun run security:pr-scan passed.
• bun run typecheck failed with many repo-wide errors outside this two-file patch; I did not see changed-line evidence tying those failures to this PR.

Recommendation: Approve. The PR is small, tested, valuable, and low risk.

[techbrewboss]

Approved. No blocking or actionable issues found; the fix is focused, tested, valuable, and low risk.