Deep merge hooksConfig/hooks settings across scopes

[acoliver]

Problem

In `packages/cli/src/config/settings.ts` line ~322, `hooksConfig` and `hooks` objects are merged across settings scopes (system, user, workspace) using shallow spread (`...spread`).

This means a higher-precedence scope that sets only one nested field (e.g., workspace sets `hooksConfig.disabled`) completely replaces the entire lower-precedence `hooksConfig` object, silently dropping other fields like `enabled` or `notifications` from the user scope.

Expected Behavior

Settings scopes should deep-merge nested objects so that workspace-level overrides only replace the specific nested fields they define, preserving unrelated fields from lower-precedence scopes.

Reproduction

1. Set user-level: `hooksConfig: { enabled: true, disabled: ['hookA'] }`
2. Set workspace-level: `hooksConfig: { disabled: [] }`
3. Observe: `enabled` is lost from the merged result

Context

Discovered during CodeRabbit review of PR #1797 (gmerge/0.26.0). Requires careful deep-merge logic and thorough testing to avoid breaking other settings.

[coderabbitai]

🔗 Related PRs

#614 - fix: resolve memory leak in SettingsDialog [merged]
#1639 - Fix LSP settings validation rejecting lsp: true (Fixes #1637) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

✅ Successfully generated implementation plan.

💡 Use @coderabbitai <feedback> to replan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Apply the same shallow object merge pattern already used for mcpServers and chatCompression to the hooks field
• This ensures each key within hooks (enabled, disabled, notifications, event-keyed hook arrays) is merged independently with last-wins semantics per key
• No new utilities or abstractions needed; the existing spread operator pattern is sufficient and consistent with codebase conventions

Implementation Steps

Implement Shallow Object Merge for Hooks

Add explicit merge handling for the hooks setting following the established pattern used for mcpServers and chatCompression, ensuring that individual fields within hooks are merged independently across scopes rather than the entire object being replaced.

Task 1: Add hooks merge handling to mergeSettings()

Update the mergeSettings() function to include explicit shallow object merge for the hooks field, preserving individual keys from lower-precedence scopes when higher-precedence scopes don't define them.

• Update packages/cli/src/config/settings.ts in the mergeSettings() function (around line 280, after the extensions block)
• Add a hooks property to the merged object using the same spread pattern as chatCompression:
  • Spread systemDefaults.hooks first (lowest precedence)
  • Then user.hooks
  • Then safeWorkspace.hooks (respects workspace trust)
  • Finally system.hooks (highest precedence override)
• Use empty object fallbacks (|| {}) for each scope to handle undefined values

Task 2: Add tests for hooks merging behavior

Add comprehensive test coverage for hooks merging across scopes to prevent regression and document expected behavior.

• Add new test cases to packages/cli/src/config/settings.test.ts within a new describe block for hooks merging
• Test cases to include:
  • User sets enabled: true, workspace sets disabled: [] → verify enabled is preserved and disabled is from workspace
  • User sets full hooks config, workspace sets partial → verify partial override behavior
  • Only user-level hooks defined → verify user values used
  • Only workspace-level hooks defined → verify workspace values used
  • System override wins over workspace and user settings
  • Untrusted workspace → verify workspace hooks are ignored
• Follow existing test patterns: mock fs.readFileSync to return appropriate JSON for each scope file path

🤖 Prompt for AI agents

This phase implements shallow object merge for the `hooks` setting in the
settings merge system, and adds corresponding test coverage. The goal is to
ensure individual keys within `hooks` are merged independently across scopes
rather than the entire object being replaced by higher-precedence scopes.

**Task 1: Update `mergeSettings()` in `packages/cli/src/config/settings.ts`**
- Locate the `mergeSettings()` function (around line 280), specifically after
the `extensions` block
- Add a `hooks` property to the `merged` object using the same shallow spread
pattern as `chatCompression`:
- Spread scopes in this order (lowest to highest precedence):
`systemDefaults.hooks`, `user.hooks`, `safeWorkspace.hooks`, `system.hooks`
- Use empty object fallbacks (`|| {}`) for each scope to safely handle undefined
values

**Task 2: Add tests in `packages/cli/src/config/settings.test.ts`**
- Add a new `describe` block specifically for hooks merging behavior
- Follow the existing test pattern of mocking `fs.readFileSync` to return
appropriate JSON per scope file path
- Cover the following test cases:
- User sets `enabled: true`, workspace sets `disabled: []` → both fields should
be present in merged result
- User sets full hooks config, workspace sets partial → workspace fields
override, user-only fields are preserved
  - Only user-level hooks defined → user values are used
  - Only workspace-level hooks defined → workspace values are used
  - System override takes precedence over both workspace and user hooks
  - Untrusted workspace → workspace hooks are ignored, user hooks are preserved

Research

The settings system in packages/cli/src/config/settings.ts uses a mergeSettings() function (lines 197-317) that combines settings from multiple scopes: schema defaults → system defaults → user → workspace → system overrides. Most scalar fields use last-wins spread, but certain fields like mcpServers, chatCompression, and includeDirectories have explicit special-case handling for object merge or array concatenation. The hooks field currently lacks this special handling and falls through to top-level spreads, causing the bug. Tests exist in packages/cli/src/config/settings.test.ts but contain no hooks-related merge tests.

---

🚀 Next Steps

🤖 All AI agent prompts combined

This phase implements shallow object merge for the `hooks` setting in the
settings merge system, and adds corresponding test coverage. The goal is to
ensure individual keys within `hooks` are merged independently across scopes
rather than the entire object being replaced by higher-precedence scopes.

**Task 1: Update `mergeSettings()` in `packages/cli/src/config/settings.ts`**
- Locate the `mergeSettings()` function (around line 280), specifically after
the `extensions` block
- Add a `hooks` property to the `merged` object using the same shallow spread
pattern as `chatCompression`:
- Spread scopes in this order (lowest to highest precedence):
`systemDefaults.hooks`, `user.hooks`, `safeWorkspace.hooks`, `system.hooks`
- Use empty object fallbacks (`|| {}`) for each scope to safely handle undefined
values

**Task 2: Add tests in `packages/cli/src/config/settings.test.ts`**
- Add a new `describe` block specifically for hooks merging behavior
- Follow the existing test pattern of mocking `fs.readFileSync` to return
appropriate JSON per scope file path
- Cover the following test cases:
- User sets `enabled: true`, workspace sets `disabled: []` → both fields should
be present in merged result
- User sets full hooks config, workspace sets partial → workspace fields
override, user-only fields are preserved
  - Only user-level hooks defined → user values are used
  - Only workspace-level hooks defined → workspace values are used
  - System override takes precedence over both workspace and user hooks
  - Untrusted workspace → workspace hooks are ignored, user hooks are preserved

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!