From 2ece2cfc9063d5d000e53869fdd62e637027b789 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sun, 12 Apr 2026 22:39:48 -0700 Subject: [PATCH] fix: stop injecting managed hooks into claude settings --- README.md | 4 +- README.zh-CN.md | 4 +- docs/ja-JP/README.md | 4 +- scripts/lib/install/apply.js | 122 ++-------------------------- tests/scripts/install-apply.test.js | 109 +++++++------------------ 5 files changed, 47 insertions(+), 196 deletions(-) diff --git a/README.md b/README.md index 45fc6568..4a235822 100644 --- a/README.md +++ b/README.md @@ -705,7 +705,9 @@ cp everything-claude-code/commands/*.md ~/.claude/commands/ #### Add hooks to settings.json -Copy the hooks from `hooks/hooks.json` to your `~/.claude/settings.json`. +Manual install only: copy the hooks from `hooks/hooks.json` to your `~/.claude/settings.json` if you are not installing ECC as a Claude plugin. + +If you installed ECC via `/plugin install`, do not copy those hooks into `settings.json`. Claude Code v2.1+ already auto-loads plugin `hooks/hooks.json`, and duplicating them in `settings.json` causes duplicate execution and `${CLAUDE_PLUGIN_ROOT}` resolution failures. #### Configure MCPs diff --git a/README.zh-CN.md b/README.zh-CN.md index 5f9f545b..7e7e9553 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -626,7 +626,9 @@ cp everything-claude-code/commands/*.md ~/.claude/commands/ ``` #### 将钩子配置添加到 settings.json -将 `hooks/hooks.json` 中的钩子配置复制到你的 `~/.claude/settings.json` 文件中。 +仅适用于手动安装：如果你没有通过 Claude 插件方式安装 ECC，可以将 `hooks/hooks.json` 中的钩子配置复制到你的 `~/.claude/settings.json` 文件中。 + +如果你是通过 `/plugin install` 安装 ECC，请不要再把这些钩子复制到 `settings.json`。Claude Code v2.1+ 会自动加载插件中的 `hooks/hooks.json`，重复注册会导致重复执行以及 `${CLAUDE_PLUGIN_ROOT}` 无法解析。 #### 配置 MCP 服务 从 `mcp-configs/mcp-servers.json` 中复制需要的 MCP 服务定义，粘贴到官方 Claude Code 配置文件 `~/.claude/settings.json` 中； diff --git a/docs/ja-JP/README.md b/docs/ja-JP/README.md index 0f8dfb0c..d7f1d5dd 100644 --- a/docs/ja-JP/README.md +++ b/docs/ja-JP/README.md @@ -497,7 +497,9 @@ cp -r everything-claude-code/skills/* ~/.claude/skills/ #### settings.json にフックを追加 -`hooks/hooks.json` のフックを `~/.claude/settings.json` にコピーします。 +手動インストール時のみ、`hooks/hooks.json` のフックを `~/.claude/settings.json` にコピーします。 + +`/plugin install` で ECC を導入した場合は、これらのフックを `settings.json` にコピーしないでください。Claude Code v2.1+ はプラグインの `hooks/hooks.json` を自動読み込みするため、二重登録すると重複実行や `${CLAUDE_PLUGIN_ROOT}` の解決失敗が発生します。 #### MCP を設定 diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index f4d66228..b74e73f3 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -46,80 +46,6 @@ function replacePluginRootPlaceholders(value, pluginRoot) { return value; } -function buildLegacyHookSignature(entry, pluginRoot) { - if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { - return null; - } - - const normalizedEntry = replacePluginRootPlaceholders(entry, pluginRoot); - - if (typeof normalizedEntry.matcher !== 'string' || !Array.isArray(normalizedEntry.hooks)) { - return null; - } - - const hookSignature = normalizedEntry.hooks.map(hook => JSON.stringify({ - type: hook && typeof hook === 'object' ? hook.type : undefined, - command: hook && typeof hook === 'object' ? hook.command : undefined, - timeout: hook && typeof hook === 'object' ? hook.timeout : undefined, - async: hook && typeof hook === 'object' ? hook.async : undefined, - })); - - return JSON.stringify({ - matcher: normalizedEntry.matcher, - hooks: hookSignature, - }); -} - -function getHookEntryAliases(entry, pluginRoot) { - const aliases = []; - - if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { - return aliases; - } - - const normalizedEntry = replacePluginRootPlaceholders(entry, pluginRoot); - - if (typeof normalizedEntry.id === 'string' && normalizedEntry.id.trim().length > 0) { - aliases.push(`id:${normalizedEntry.id.trim()}`); - } - - const legacySignature = buildLegacyHookSignature(normalizedEntry, pluginRoot); - if (legacySignature) { - aliases.push(`legacy:${legacySignature}`); - } - - aliases.push(`json:${JSON.stringify(normalizedEntry)}`); - - return aliases; -} - -function mergeHookEntries(existingEntries, incomingEntries, pluginRoot) { - const mergedEntries = []; - const seenEntries = new Set(); - - for (const entry of [...existingEntries, ...incomingEntries]) { - if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { - continue; - } - - if ('id' in entry && typeof entry.id !== 'string') { - continue; - } - - const aliases = getHookEntryAliases(entry, pluginRoot); - if (aliases.some(alias => seenEntries.has(alias))) { - continue; - } - - for (const alias of aliases) { - seenEntries.add(alias); - } - mergedEntries.push(replacePluginRootPlaceholders(entry, pluginRoot)); - } - - return mergedEntries; -} - function findHooksSourcePath(plan, hooksDestinationPath) { const operation = plan.operations.find(item => item.destinationPath === hooksDestinationPath); return operation ? operation.sourcePath : null; @@ -168,7 +94,7 @@ function buildFilteredMcpWrites(plan) { return writes; } -function buildMergedSettings(plan) { +function buildResolvedClaudeHooks(plan) { if (!plan.adapter || plan.adapter.target !== 'claude') { return null; } @@ -181,46 +107,22 @@ function buildMergedSettings(plan) { } const hooksConfig = readJsonObject(hooksSourcePath, 'hooks config'); - const incomingHooks = replacePluginRootPlaceholders(hooksConfig.hooks, pluginRoot); - if (!incomingHooks || typeof incomingHooks !== 'object' || Array.isArray(incomingHooks)) { + const resolvedHooks = replacePluginRootPlaceholders(hooksConfig.hooks, pluginRoot); + if (!resolvedHooks || typeof resolvedHooks !== 'object' || Array.isArray(resolvedHooks)) { 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, pluginRoot); - } - - const mergedSettings = { - ...settings, - hooks: mergedHooks, - }; - return { - settingsPath, - mergedSettings, hooksDestinationPath, resolvedHooksConfig: { ...hooksConfig, - hooks: incomingHooks, + hooks: resolvedHooks, }, }; } function applyInstallPlan(plan) { - const mergedSettingsPlan = buildMergedSettings(plan); + const resolvedClaudeHooksPlan = buildResolvedClaudeHooks(plan); const filteredMcpWrites = buildFilteredMcpWrites(plan); for (const operation of plan.operations) { @@ -228,17 +130,11 @@ function applyInstallPlan(plan) { fs.copyFileSync(operation.sourcePath, operation.destinationPath); } - if (mergedSettingsPlan) { - fs.mkdirSync(path.dirname(mergedSettingsPlan.hooksDestinationPath), { recursive: true }); + if (resolvedClaudeHooksPlan) { + fs.mkdirSync(path.dirname(resolvedClaudeHooksPlan.hooksDestinationPath), { recursive: true }); fs.writeFileSync( - mergedSettingsPlan.hooksDestinationPath, - JSON.stringify(mergedSettingsPlan.resolvedHooksConfig, null, 2) + '\n', - 'utf8' - ); - fs.mkdirSync(path.dirname(mergedSettingsPlan.settingsPath), { recursive: true }); - fs.writeFileSync( - mergedSettingsPlan.settingsPath, - JSON.stringify(mergedSettingsPlan.mergedSettings, null, 2) + '\n', + resolvedClaudeHooksPlan.hooksDestinationPath, + JSON.stringify(resolvedClaudeHooksPlan.resolvedHooksConfig, null, 2) + '\n', 'utf8' ); } diff --git a/tests/scripts/install-apply.test.js b/tests/scripts/install-apply.test.js index 91b162ae..b9eb0033 100644 --- a/tests/scripts/install-apply.test.js +++ b/tests/scripts/install-apply.test.js @@ -331,7 +331,7 @@ 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', () => { + if (test('installs claude hooks without generating settings.json', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -341,15 +341,7 @@ function runTests() { 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'); + assert.ok(!fs.existsSync(path.join(claudeRoot, 'settings.json')), 'settings.json should not be created just to install managed hooks'); } finally { cleanup(homeDir); cleanup(projectDir); @@ -365,23 +357,11 @@ function runTests() { assert.strictEqual(result.code, 0, result.stderr); const claudeRoot = path.join(homeDir, '.claude'); - const settings = readJson(path.join(claudeRoot, 'settings.json')); const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json')); const normSep = (s) => s.replace(/\\/g, '/'); const expectedFragment = normSep(path.join(claudeRoot, 'scripts', 'hooks', 'auto-tmux-dev.js')); - const autoTmuxEntry = settings.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev'); - assert.ok(autoTmuxEntry, 'settings.json should include the auto tmux hook'); - assert.ok( - normSep(autoTmuxEntry.hooks[0].command).includes(expectedFragment), - 'settings.json should use the installed Claude root for hook commands' - ); - assert.ok( - !autoTmuxEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'), - 'settings.json should not retain CLAUDE_PLUGIN_ROOT placeholders after install' - ); - const installedAutoTmuxEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev'); assert.ok(installedAutoTmuxEntry, 'hooks/hooks.json should include the auto tmux hook'); assert.ok( @@ -398,7 +378,7 @@ function runTests() { } })) passed++; else failed++; - if (test('preserves existing settings fields and hook entries when merging hooks', () => { + if (test('preserves existing settings.json without mutating it during claude install', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -423,17 +403,15 @@ function runTests() { 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' + 'existing hooks should be left untouched' + ); + assert.deepStrictEqual( + settings.hooks.PreToolUse, + [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }], + 'managed Claude hooks should not be injected into settings.json' ); } finally { cleanup(homeDir); @@ -515,7 +493,7 @@ function runTests() { } })) passed++; else failed++; - if (test('reinstall does not duplicate managed hook entries', () => { + if (test('reinstall does not create settings.json when only managed hooks are installed', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -523,69 +501,43 @@ function runTests() { 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' - ); + assert.ok(!fs.existsSync(path.join(homeDir, '.claude', 'settings.json'))); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('reinstall deduplicates legacy hooks without ids against new managed ids', () => { + if (test('reinstall leaves pre-existing hook-based settings.json untouched', () => { 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 legacySettings = JSON.parse(JSON.stringify(afterFirstInstall)); - - for (const entries of Object.values(legacySettings.hooks)) { - if (!Array.isArray(entries)) { - continue; - } - for (const entry of entries) { - delete entry.id; - } - } - + const claudeRoot = path.join(homeDir, '.claude'); + fs.mkdirSync(claudeRoot, { recursive: true }); + const settingsPath = path.join(claudeRoot, 'settings.json'); + const legacySettings = { + hooks: { + PreToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo legacy-pretool' }] }], + }, + }; fs.writeFileSync(settingsPath, JSON.stringify(legacySettings, null, 2)); - const legacyPreToolUseLength = legacySettings.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, - legacyPreToolUseLength, - 'legacy hook installs should not duplicate when ids are introduced' - ); - assert.ok( - afterSecondInstall.hooks.PreToolUse.every(entry => entry && typeof entry === 'object'), - 'merged hook entries should remain valid objects' - ); + assert.deepStrictEqual(afterSecondInstall, legacySettings); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('fails when existing settings.json is malformed', () => { + if (test('ignores malformed existing settings.json during claude install', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -596,18 +548,17 @@ function runTests() { 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(result.code, 0, result.stderr); 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'); + assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); + assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); } finally { cleanup(homeDir); cleanup(projectDir); } })) passed++; else failed++; - if (test('fails when existing settings.json root is not an object', () => { + if (test('ignores non-object existing settings.json during claude install', () => { const homeDir = createTempDir('install-apply-home-'); const projectDir = createTempDir('install-apply-project-'); @@ -618,12 +569,10 @@ function runTests() { 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(result.code, 0, result.stderr); 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'); + assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied'); + assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written'); } finally { cleanup(homeDir); cleanup(projectDir);