From 9f37a5d8c76f42e19a75acd987e2c19cd7660a9e Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 27 Mar 2026 07:52:03 -0400 Subject: [PATCH 1/3] fix(installer): preserve existing claude hook settings --- scripts/lib/install/apply.js | 73 +++++++++++++++++- tests/scripts/install-apply.test.js | 115 ++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 567d4a78..26fce135 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -1,15 +1,86 @@ 'use strict'; const fs = require('fs'); +const path = require('path'); const { writeInstallState } = require('../install-state'); +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 mergeHooksIntoSettings(plan) { + if (!plan.adapter || plan.adapter.target !== 'claude') { + return; + } + + const hooksJsonPath = path.join(plan.targetRoot, 'hooks', 'hooks.json'); + if (!fs.existsSync(hooksJsonPath)) { + return; + } + + let hooksConfig; + try { + hooksConfig = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + } catch (error) { + throw new Error(`Failed to parse hooks config at ${hooksJsonPath}: ${error.message}`); + } + + const incomingHooks = hooksConfig.hooks; + if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) { + return; + } + + const settingsPath = path.join(plan.targetRoot, 'settings.json'); + let settings = {}; + if (fs.existsSync(settingsPath)) { + try { + settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + } catch (error) { + throw new Error(`Failed to parse existing settings at ${settingsPath}: ${error.message}`); + } + } + + 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, + }; + + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2) + '\n', 'utf8'); +} + function applyInstallPlan(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); } + mergeHooksIntoSettings(plan); writeInstallState(plan.installStatePath, plan.statePreview); return { diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index 39743bfc..13b9d7fa 100644 --- a/tests/scripts/install-apply.test.js +++ b/tests/scripts/install-apply.test.js @@ -326,6 +326,121 @@ 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'); + } finally { + 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-'); From d7e6bb242aedbe83b712ac13cefb0e3c06c6580d Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 27 Mar 2026 07:59:19 -0400 Subject: [PATCH 2/3] fix(installer): reject invalid claude settings roots --- scripts/lib/install/apply.js | 3 +++ tests/scripts/install-apply.test.js | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 26fce135..857a1617 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -49,6 +49,9 @@ function mergeHooksIntoSettings(plan) { if (fs.existsSync(settingsPath)) { try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + if (!settings || typeof settings !== 'object' || Array.isArray(settings)) { + throw new Error('root value must be a JSON object'); + } } catch (error) { throw new Error(`Failed to parse existing settings at ${settingsPath}: ${error.message}`); } diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index 13b9d7fa..b82d67aa 100644 --- a/tests/scripts/install-apply.test.js +++ b/tests/scripts/install-apply.test.js @@ -441,6 +441,27 @@ function runTests() { } })) 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('Failed to parse existing settings at')); + assert.ok(result.stderr.includes('root value must be a JSON object')); + assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n'); + } finally { + 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-'); From 47aa415b06f32f5f875c6321c0f93db9a686eaaf Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sat, 28 Mar 2026 19:55:28 -0400 Subject: [PATCH 3/3] fix(installer): validate hooks and settings before install --- scripts/ecc.js | 0 scripts/install-apply.js | 0 scripts/lib/install/apply.js | 68 +++++++++++++++++++---------- tests/scripts/install-apply.test.js | 34 ++++++++++++++- 4 files changed, 76 insertions(+), 26 deletions(-) mode change 100644 => 100755 scripts/ecc.js mode change 100644 => 100755 scripts/install-apply.js diff --git a/scripts/ecc.js b/scripts/ecc.js old mode 100644 new mode 100755 diff --git a/scripts/install-apply.js b/scripts/install-apply.js old mode 100644 new mode 100755 diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 857a1617..bab5b6d5 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -5,6 +5,21 @@ 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(); @@ -22,39 +37,32 @@ function mergeHookEntries(existingEntries, incomingEntries) { return mergedEntries; } -function mergeHooksIntoSettings(plan) { +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; + return null; } - const hooksJsonPath = path.join(plan.targetRoot, 'hooks', 'hooks.json'); - if (!fs.existsSync(hooksJsonPath)) { - return; - } - - let hooksConfig; - try { - hooksConfig = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); - } catch (error) { - throw new Error(`Failed to parse hooks config at ${hooksJsonPath}: ${error.message}`); + 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)) { - return; + 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)) { - try { - settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); - if (!settings || typeof settings !== 'object' || Array.isArray(settings)) { - throw new Error('root value must be a JSON object'); - } - } catch (error) { - throw new Error(`Failed to parse existing settings at ${settingsPath}: ${error.message}`); - } + settings = readJsonObject(settingsPath, 'existing settings'); } const existingHooks = settings.hooks && typeof settings.hooks === 'object' && !Array.isArray(settings.hooks) @@ -73,17 +81,29 @@ function mergeHooksIntoSettings(plan) { hooks: mergedHooks, }; - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync(settingsPath, JSON.stringify(mergedSettings, null, 2) + '\n', 'utf8'); + return { + settingsPath, + mergedSettings, + }; } function applyInstallPlan(plan) { + const mergedSettingsPlan = buildMergedSettings(plan); + for (const operation of plan.operations) { fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true }); fs.copyFileSync(operation.sourcePath, operation.destinationPath); } - mergeHooksIntoSettings(plan); + 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 { diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index b82d67aa..1416489c 100644 --- a/tests/scripts/install-apply.test.js +++ b/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, @@ -435,6 +437,8 @@ function runTests() { 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); @@ -453,15 +457,41 @@ function runTests() { 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.ok(result.stderr.includes('root value must be a JSON object')); + 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-');