Merge pull request #964 from affaan-m/fix/claude-hooks-settings-merge-safe

fix(installer): preserve existing Claude hook settings

scripts/lib/install/apply.js

@@ -1,15 +1,109 @@
 'use strict';
 
 const fs = require('fs');
+const path = require('path');
 
 const { writeInstallState } = require('../install-state');
 
+function readJsonObject(filePath, label) {
+  let parsed;
+  try {
+    parsed = JSON.parse(fs.readFileSync(filePath, 'utf8'));
+  } catch (error) {
+    throw new Error(`Failed to parse ${label} at ${filePath}: ${error.message}`);
+  }
+
+  if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
+    throw new Error(`Invalid ${label} at ${filePath}: expected a JSON object`);
+  }
+
+  return parsed;
+}
+
+function mergeHookEntries(existingEntries, incomingEntries) {
+  const mergedEntries = [];
+  const seenEntries = new Set();
+
+  for (const entry of [...existingEntries, ...incomingEntries]) {
+    const entryKey = JSON.stringify(entry);
+    if (seenEntries.has(entryKey)) {
+      continue;
+    }
+
+    seenEntries.add(entryKey);
+    mergedEntries.push(entry);
+  }
+
+  return mergedEntries;
+}
+
+function findHooksSourcePath(plan, hooksDestinationPath) {
+  const operation = plan.operations.find(item => item.destinationPath === hooksDestinationPath);
+  return operation ? operation.sourcePath : null;
+}
+
+function buildMergedSettings(plan) {
+  if (!plan.adapter || plan.adapter.target !== 'claude') {
+    return null;
+  }
+
+  const hooksDestinationPath = path.join(plan.targetRoot, 'hooks', 'hooks.json');
+  const hooksSourcePath = findHooksSourcePath(plan, hooksDestinationPath) || hooksDestinationPath;
+  if (!fs.existsSync(hooksSourcePath)) {
+    return null;
+  }
+
+  const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config');
+  const incomingHooks = hooksConfig.hooks;
+  if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) {
+    throw new Error(`Invalid hooks config at ${hooksSourcePath}: expected "hooks" to be a JSON object`);
+  }
+
+  const settingsPath = path.join(plan.targetRoot, 'settings.json');
+  let settings = {};
+  if (fs.existsSync(settingsPath)) {
+    settings = readJsonObject(settingsPath, 'existing settings');
+  }
+
+  const existingHooks = settings.hooks && typeof settings.hooks === 'object' && !Array.isArray(settings.hooks)
+    ? settings.hooks
+    : {};
+  const mergedHooks = { ...existingHooks };
+
+  for (const [eventName, incomingEntries] of Object.entries(incomingHooks)) {
+    const currentEntries = Array.isArray(existingHooks[eventName]) ? existingHooks[eventName] : [];
+    const nextEntries = Array.isArray(incomingEntries) ? incomingEntries : [];
+    mergedHooks[eventName] = mergeHookEntries(currentEntries, nextEntries);
+  }
+
+  const mergedSettings = {
+    ...settings,
+    hooks: mergedHooks,
+  };
+
+  return {
+    settingsPath,
+    mergedSettings,
+  };
+}
+
 function applyInstallPlan(plan) {
+  const mergedSettingsPlan = buildMergedSettings(plan);
+
   for (const operation of plan.operations) {
-    fs.mkdirSync(require('path').dirname(operation.destinationPath), { recursive: true });
+    fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true });
     fs.copyFileSync(operation.sourcePath, operation.destinationPath);
   }
 
+  if (mergedSettingsPlan) {
+    fs.mkdirSync(path.dirname(mergedSettingsPlan.settingsPath), { recursive: true });
+    fs.writeFileSync(
+      mergedSettingsPlan.settingsPath,
+      JSON.stringify(mergedSettingsPlan.mergedSettings, null, 2) + '\n',
+      'utf8'
+    );
+  }
+
   writeInstallState(plan.installStatePath, plan.statePreview);
 
   return {

tests/scripts/install-apply.test.js

@@ -22,6 +22,8 @@ function readJson(filePath) {
   return JSON.parse(fs.readFileSync(filePath, 'utf8'));
 }
 
+const REPO_ROOT = path.join(__dirname, '..', '..');
+
 function run(args = [], options = {}) {
   const env = {
     ...process.env,
@@ -326,6 +328,170 @@ function runTests() {
     assert.ok(result.stderr.includes('Unknown install module: ghost-module'));
   })) passed++; else failed++;
 
+  if (test('merges hooks into settings.json for claude target install', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+
+    try {
+      const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(result.code, 0, result.stderr);
+
+      const claudeRoot = path.join(homeDir, '.claude');
+      assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should be copied');
+
+      const settingsPath = path.join(claudeRoot, 'settings.json');
+      assert.ok(fs.existsSync(settingsPath), 'settings.json should exist after install');
+
+      const settings = readJson(settingsPath);
+      assert.ok(settings.hooks, 'settings.json should contain hooks key');
+      assert.ok(settings.hooks.PreToolUse, 'hooks should include PreToolUse');
+      assert.ok(Array.isArray(settings.hooks.PreToolUse), 'PreToolUse should be an array');
+      assert.ok(settings.hooks.PreToolUse.length > 0, 'PreToolUse should have entries');
+    } finally {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
+  if (test('preserves existing settings fields and hook entries when merging hooks', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+
+    try {
+      const claudeRoot = path.join(homeDir, '.claude');
+      fs.mkdirSync(claudeRoot, { recursive: true });
+      fs.writeFileSync(
+        path.join(claudeRoot, 'settings.json'),
+        JSON.stringify({
+          effortLevel: 'high',
+          env: { MY_VAR: '1' },
+          hooks: {
+            PreToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }],
+            UserPromptSubmit: [{ matcher: '*', hooks: [{ type: 'command', command: 'echo custom-submit' }] }],
+          },
+        }, null, 2)
+      );
+
+      const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(result.code, 0, result.stderr);
+
+      const settings = readJson(path.join(claudeRoot, 'settings.json'));
+      assert.strictEqual(settings.effortLevel, 'high', 'existing effortLevel should be preserved');
+      assert.deepStrictEqual(settings.env, { MY_VAR: '1' }, 'existing env should be preserved');
+      assert.ok(settings.hooks, 'hooks should be merged in');
+      assert.ok(settings.hooks.PreToolUse, 'PreToolUse hooks should exist');
+      assert.ok(
+        settings.hooks.PreToolUse.some(entry => JSON.stringify(entry).includes('echo custom-pretool')),
+        'existing PreToolUse entries should be preserved'
+      );
+      assert.ok(settings.hooks.PreToolUse.length > 1, 'ECC PreToolUse hooks should be appended');
+      assert.deepStrictEqual(
+        settings.hooks.UserPromptSubmit,
+        [{ matcher: '*', hooks: [{ type: 'command', command: 'echo custom-submit' }] }],
+        'user-defined hook event types should be preserved'
+      );
+    } finally {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
+  if (test('reinstall does not duplicate managed hook entries', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+
+    try {
+      const firstInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(firstInstall.code, 0, firstInstall.stderr);
+
+      const settingsPath = path.join(homeDir, '.claude', 'settings.json');
+      const afterFirstInstall = readJson(settingsPath);
+      const preToolUseLength = afterFirstInstall.hooks.PreToolUse.length;
+
+      const secondInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(secondInstall.code, 0, secondInstall.stderr);
+
+      const afterSecondInstall = readJson(settingsPath);
+      assert.strictEqual(
+        afterSecondInstall.hooks.PreToolUse.length,
+        preToolUseLength,
+        'managed hook entries should not duplicate on reinstall'
+      );
+    } finally {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
+  if (test('fails when existing settings.json is malformed', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+
+    try {
+      const claudeRoot = path.join(homeDir, '.claude');
+      fs.mkdirSync(claudeRoot, { recursive: true });
+      const settingsPath = path.join(claudeRoot, 'settings.json');
+      fs.writeFileSync(settingsPath, '{ invalid json\n');
+
+      const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(result.code, 1);
+      assert.ok(result.stderr.includes('Failed to parse existing settings at'));
+      assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
+    } finally {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
+  if (test('fails when existing settings.json root is not an object', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+
+    try {
+      const claudeRoot = path.join(homeDir, '.claude');
+      fs.mkdirSync(claudeRoot, { recursive: true });
+      const settingsPath = path.join(claudeRoot, 'settings.json');
+      fs.writeFileSync(settingsPath, '[]\n');
+
+      const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(result.code, 1);
+      assert.ok(result.stderr.includes('Invalid existing settings at'));
+      assert.ok(result.stderr.includes('expected a JSON object'));
+      assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
+    } finally {
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
+  if (test('fails when source hooks.json root is not an object before copying files', () => {
+    const homeDir = createTempDir('install-apply-home-');
+    const projectDir = createTempDir('install-apply-project-');
+    const sourceHooksPath = path.join(REPO_ROOT, 'hooks', 'hooks.json');
+    const originalHooks = fs.readFileSync(sourceHooksPath, 'utf8');
+
+    try {
+      fs.writeFileSync(sourceHooksPath, '[]\n');
+
+      const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
+      assert.strictEqual(result.code, 1);
+      assert.ok(result.stderr.includes('Invalid hooks config at'));
+      assert.ok(result.stderr.includes('expected a JSON object'));
+
+      const claudeRoot = path.join(homeDir, '.claude');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied when source hooks are invalid');
+      assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written when source hooks are invalid');
+    } finally {
+      fs.writeFileSync(sourceHooksPath, originalHooks);
+      cleanup(homeDir);
+      cleanup(projectDir);
+    }
+  })) passed++; else failed++;
+
   if (test('installs from ecc-install.json and persists component selections', () => {
     const homeDir = createTempDir('install-apply-home-');
     const projectDir = createTempDir('install-apply-project-');