Merge pull request #1389 from affaan-m/fix/hook-plugin-root-resolution

fix: stop duplicating managed Claude hooks into settings
2026-04-13 21:33:32 +08:00 · 2026-04-12 23:00:04 -07:00
parent 28edd197c2 f4c7aac5b8
commit e29da39eaf
6 changed files with 102 additions and 213 deletions
--- 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

--- 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` 中；
--- 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 を設定

--- a/docs/zh-TW/README.md
+++ b/docs/zh-TW/README.md
@@ -318,7 +318,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

--- 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'
    );
  }
--- a/tests/scripts/install-apply.test.js
+++ b/tests/scripts/install-apply.test.js
@@ -23,8 +23,6 @@ function readJson(filePath) {
  return JSON.parse(fs.readFileSync(filePath, 'utf8'));
 }

-const REPO_ROOT = path.join(__dirname, '..', '..');
-
 function run(args = [], options = {}) {
  const homeDir = options.homeDir || process.env.HOME;
  const env = {
@@ -331,7 +329,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 +339,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 +355,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 +376,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 +401,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 +491,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 +499,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 +546,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 +567,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);
@@ -631,26 +578,64 @@ function runTests() {
  })) 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');
+    const tempDir = createTempDir('install-apply-invalid-hooks-');
+    const targetRoot = path.join(tempDir, '.claude');
+    const installStatePath = path.join(targetRoot, 'ecc', 'install-state.json');
+    const sourceHooksPath = path.join(tempDir, 'hooks.json');

    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'));
+      assert.throws(() => {
+        applyInstallPlan({
+          targetRoot,
+          installStatePath,
+          statePreview: {
+            schemaVersion: 'ecc.install.v1',
+            installedAt: new Date().toISOString(),
+            target: {
+              id: 'claude-home',
+              kind: 'home',
+              root: targetRoot,
+              installStatePath,
+            },
+            request: {
+              profile: 'core',
+              modules: [],
+              includeComponents: [],
+              excludeComponents: [],
+              legacyLanguages: [],
+              legacyMode: false,
+            },
+            resolution: {
+              selectedModules: ['hooks-runtime'],
+              skippedModules: [],
+            },
+            source: {
+              repoVersion: null,
+              repoCommit: null,
+              manifestVersion: 1,
+            },
+            operations: [],
+          },
+          adapter: { target: 'claude' },
+          operations: [{
+            kind: 'copy-file',
+            moduleId: 'hooks-runtime',
+            sourcePath: sourceHooksPath,
+            sourceRelativePath: 'hooks/hooks.json',
+            destinationPath: path.join(targetRoot, 'hooks', 'hooks.json'),
+            strategy: 'preserve-relative-path',
+            ownership: 'managed',
+            scaffoldOnly: false,
+          }],
+        });
+      }, /Invalid hooks config at .*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');
+      assert.ok(!fs.existsSync(path.join(targetRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied when source hooks are invalid');
+      assert.ok(!fs.existsSync(installStatePath), 'install state should not be written when source hooks are invalid');
    } finally {
-      fs.writeFileSync(sourceHooksPath, originalHooks);
-      cleanup(homeDir);
-      cleanup(projectDir);
+      cleanup(tempDir);
    }
  })) passed++; else failed++;